Re: [PATCH 3/5] mtd: onenand: samsung: Unify resource order for controller variants

From: Krzysztof Kozlowski
Date: Sun Apr 24 2022 - 07:05:21 EST


On 23/04/2022 05:46, Jonathan Bakker wrote:
> From: Tomasz Figa <tomasz.figa@xxxxxxxxx>
>
> Before this patch, the order of memory resources requested by the driver
> was controller base as first and OneNAND chip base as second for
> S3C64xx/S5PC100 variant and the opposite for S5PC110/S5PV210 variant.
>
> To make this more consistent, this patch swaps the order of resources
> for the latter and updates platform code accordingly. As a nice side
> effect there is a slight reduction in line count of probe function.
>
> This will make the transition to DT-based probing much easier.
>
> Signed-off-by: Tomasz Figa <tomasz.figa@xxxxxxxxx>
> Signed-off-by: Paweł Chmiel <pawel.mikolaj.chmiel@xxxxxxxxx>
> Signed-off-by: Jonathan Bakker <xc-racer2@xxxxxxx>
> ---
> drivers/mtd/nand/onenand/onenand_samsung.c | 48 ++++++++++------------
> 1 file changed, 22 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/mtd/nand/onenand/onenand_samsung.c b/drivers/mtd/nand/onenand/onenand_samsung.c
> index 924f5ddc9505..a3ef4add865a 100644
> --- a/drivers/mtd/nand/onenand/onenand_samsung.c
> +++ b/drivers/mtd/nand/onenand/onenand_samsung.c
> @@ -123,14 +123,13 @@ struct s3c_onenand {
> struct mtd_info *mtd;
> struct platform_device *pdev;
> enum soc_type type;
> - void __iomem *base;
> - void __iomem *ahb_addr;
> + void __iomem *ctrl_base;
> + void __iomem *chip_base;
> int bootram_command;
> void *page_buf;
> void *oob_buf;
> unsigned int (*mem_addr)(int fba, int fpa, int fsa);
> unsigned int (*cmd_map)(unsigned int type, unsigned int val);
> - void __iomem *dma_addr;
> unsigned long phys_base;
> struct completion complete;
> };
> @@ -144,22 +143,22 @@ static struct s3c_onenand *onenand;
>
> static inline int s3c_read_reg(int offset)
> {
> - return readl(onenand->base + offset);
> + return readl(onenand->ctrl_base + offset);
> }
>
> static inline void s3c_write_reg(int value, int offset)
> {
> - writel(value, onenand->base + offset);
> + writel(value, onenand->ctrl_base + offset);
> }
>
> static inline int s3c_read_cmd(unsigned int cmd)
> {
> - return readl(onenand->ahb_addr + cmd);
> + return readl(onenand->chip_base + cmd);
> }
>
> static inline void s3c_write_cmd(int value, unsigned int cmd)
> {
> - writel(value, onenand->ahb_addr + cmd);
> + writel(value, onenand->chip_base + cmd);
> }
>
> #ifdef SAMSUNG_DEBUG
> @@ -517,7 +516,7 @@ static int (*s5pc110_dma_ops)(dma_addr_t dst, dma_addr_t src, size_t count, int
>
> static int s5pc110_dma_poll(dma_addr_t dst, dma_addr_t src, size_t count, int direction)
> {
> - void __iomem *base = onenand->dma_addr;
> + void __iomem *base = onenand->ctrl_base;
> int status;
> unsigned long timeout;
>
> @@ -561,7 +560,7 @@ static int s5pc110_dma_poll(dma_addr_t dst, dma_addr_t src, size_t count, int di
>
> static irqreturn_t s5pc110_onenand_irq(int irq, void *data)
> {
> - void __iomem *base = onenand->dma_addr;
> + void __iomem *base = onenand->ctrl_base;

This is confusing a bit. Before dma_addr was IORESOURCE_MEM no. 1, now
it is no. 0, so it's reversed for S5P. However DTS is not updated, so
what's there? What does the DTS follow here?

Best regards,
Krzysztof