linux-kernel - Re: [PATCH 1/9] mtd: rawnand: brcmnand: Allow SoC to provide I/O operations

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20220103174953.40d7fa52@xps13>
Date:   Mon, 3 Jan 2022 17:49:53 +0100
From:   Miquel Raynal <miquel.raynal@...tlin.com>
To:     Florian Fainelli <f.fainelli@...il.com>
Cc:     linux-mtd@...ts.infradead.org,
        Rafał Miłecki <zajec5@...il.com>,
        Richard Weinberger <richard@....at>,
        Vignesh Raghavendra <vigneshr@...com>,
        Brian Norris <computersforpeace@...il.com>,
        Kamal Dasu <kdasu.kdev@...il.com>,
        Arnd Bergmann <arnd@...db.de>,
        Cai Huoqing <caihuoqing@...du.com>,
        Colin Ian King <colin.king@...el.com>,
        linux-kernel@...r.kernel.org (open list),
        linux-wireless@...r.kernel.org (open list:BROADCOM SPECIFIC AMBA DRIVER
        (BCMA)),
        bcm-kernel-feedback-list@...adcom.com (open list:BROADCOM STB NAND
        FLASH DRIVER)
Subject: Re: [PATCH 1/9] mtd: rawnand: brcmnand: Allow SoC to provide I/O
 operations

Hi Florian,

f.fainelli@...il.com wrote on Wed, 22 Dec 2021 16:22:17 -0800:

> Allow a brcmnand_soc instance to provide a custom set of I/O operations
> which we will require when using this driver on a BCMA bus which is not
> directly memory mapped I/O. Update the nand_{read,write}_reg accordingly
> to use the SoC operations if provided.
> 
> Signed-off-by: Florian Fainelli <f.fainelli@...il.com>
> ---
>  drivers/mtd/nand/raw/brcmnand/brcmnand.c | 14 ++++++++++++--
>  drivers/mtd/nand/raw/brcmnand/brcmnand.h | 23 +++++++++++++++++++++++
>  2 files changed, 35 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> index f75929783b94..7a1673b1b1af 100644
> --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> @@ -594,13 +594,18 @@ enum {
>  
>  static inline u32 nand_readreg(struct brcmnand_controller *ctrl, u32 offs)
>  {
> +	if (brcmnand_soc_has_ops(ctrl->soc))
> +		return brcmnand_soc_read(ctrl->soc, offs);
>  	return brcmnand_readl(ctrl->nand_base + offs);
>  }
>  
>  static inline void nand_writereg(struct brcmnand_controller *ctrl, u32 offs,
>  				 u32 val)
>  {
> -	brcmnand_writel(val, ctrl->nand_base + offs);
> +	if (brcmnand_soc_has_ops(ctrl->soc))
> +		brcmnand_soc_write(ctrl->soc, val, offs);
> +	else
> +		brcmnand_writel(val, ctrl->nand_base + offs);
>  }
>  
>  static int brcmnand_revision_init(struct brcmnand_controller *ctrl)
> @@ -766,13 +771,18 @@ static inline void brcmnand_rmw_reg(struct brcmnand_controller *ctrl,
>  
>  static inline u32 brcmnand_read_fc(struct brcmnand_controller *ctrl, int word)
>  {
> +	if (brcmnand_soc_has_ops(ctrl->soc))
> +		return brcmnand_soc_read(ctrl->soc, ~0);
>  	return __raw_readl(ctrl->nand_fc + word * 4);
>  }
>  
>  static inline void brcmnand_write_fc(struct brcmnand_controller *ctrl,
>  				     int word, u32 val)
>  {
> -	__raw_writel(val, ctrl->nand_fc + word * 4);
> +	if (brcmnand_soc_has_ops(ctrl->soc))
> +		brcmnand_soc_write(ctrl->soc, val, ~0);
> +	else
> +		__raw_writel(val, ctrl->nand_fc + word * 4);
>  }
>  
>  static inline void edu_writel(struct brcmnand_controller *ctrl,
> diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.h b/drivers/mtd/nand/raw/brcmnand/brcmnand.h
> index eb498fbe505e..a3f2ad5f6572 100644
> --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.h
> +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.h
> @@ -11,12 +11,19 @@
>  
>  struct platform_device;
>  struct dev_pm_ops;
> +struct brcmnand_io_ops;
>  
>  struct brcmnand_soc {
>  	bool (*ctlrdy_ack)(struct brcmnand_soc *soc);
>  	void (*ctlrdy_set_enabled)(struct brcmnand_soc *soc, bool en);
>  	void (*prepare_data_bus)(struct brcmnand_soc *soc, bool prepare,
>  				 bool is_param);
> +	const struct brcmnand_io_ops *ops;
> +};
> +
> +struct brcmnand_io_ops {
> +	u32 (*read_reg)(struct brcmnand_soc *soc, u32 offset);
> +	void (*write_reg)(struct brcmnand_soc *soc, u32 val, u32 offset);
>  };
>  
>  static inline void brcmnand_soc_data_bus_prepare(struct brcmnand_soc *soc,
> @@ -58,6 +65,22 @@ static inline void brcmnand_writel(u32 val, void __iomem *addr)
>  		writel_relaxed(val, addr);
>  }
>  
> +static inline bool brcmnand_soc_has_ops(struct brcmnand_soc *soc)
> +{
> +	return soc && soc->ops && soc->ops->read_reg && soc->ops->write_reg;
> +}
> +
> +static inline u32 brcmnand_soc_read(struct brcmnand_soc *soc, u32 offset)
> +{
> +	return soc->ops->read_reg(soc, offset);
> +}
> +
> +static inline void brcmnand_soc_write(struct brcmnand_soc *soc, u32 val,
> +				      u32 offset)
> +{
> +	soc->ops->write_reg(soc, val, offset);
> +}
> +

It might be worth looking into more optimized ways to do these checks,
in particular the read/write_reg ones because you're checking against
some static data which cannot be optimized out by the compiler but
won't change in the lifetime of the kernel.

Thanks,
Miquèl