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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <b6109c94-d4c4-6694-90bd-54f5c4fb9383@linaro.org>
Date:   Sun, 24 Apr 2022 13:05:11 +0200
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To:     Jonathan Bakker <xc-racer2@...e.ca>,
        Miquel Raynal <miquel.raynal@...tlin.com>
Cc:     Kyungmin Park <kyungmin.park@...sung.com>,
        Richard Weinberger <richard@....at>,
        Vignesh Raghavendra <vigneshr@...com>,
        linux-mtd@...ts.infradead.org, linux-kernel@...r.kernel.org,
        linux-samsung-soc@...r.kernel.org,
        Tomasz Figa <tomasz.figa@...il.com>,
        Paweł Chmiel <pawel.mikolaj.chmiel@...il.com>
Subject: Re: [PATCH 3/5] mtd: onenand: samsung: Unify resource order for
 controller variants

On 23/04/2022 05:46, Jonathan Bakker wrote:
> From: Tomasz Figa <tomasz.figa@...il.com>
> 
> 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@...il.com>
> Signed-off-by: Paweł Chmiel <pawel.mikolaj.chmiel@...il.com>
> Signed-off-by: Jonathan Bakker <xc-racer2@...e.ca>
> ---
>  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