Re: [PATCH 9/9] mtd: rawnand: brcmnand: Add BCMA shim

[Florian Fainelli <f.fainelli@gmail.com>]

On 1/3/2022 9:06 AM, Miquel Raynal wrote:
> Hi Florian,
> 
> f.fainelli@gmail.com wrote on Wed, 22 Dec 2021 16:22:25 -0800:
> 
>> Add a BCMA shim to allow us to register the brcmnand driver using the
>> BCMA bus which provides indirect memory mapped access to SoC registers.
>>
>> There are a number of registers that need to be byte swapped because
>> they are natively big endian, coming directly from the NAND chip, and
>> there is no bus interface unlike the iProc or STB platforms that
>> performs the byte swapping for us.
>>
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>> ---
>>   drivers/mtd/nand/raw/Kconfig              |  11 ++
>>   drivers/mtd/nand/raw/brcmnand/Makefile    |   2 +
>>   drivers/mtd/nand/raw/brcmnand/bcma_nand.c | 131 ++++++++++++++++++++++
>>   3 files changed, 144 insertions(+)
>>   create mode 100644 drivers/mtd/nand/raw/brcmnand/bcma_nand.c
>>
>> diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig
>> index 0a45d3c6c15b..f643e02e5559 100644
>> --- a/drivers/mtd/nand/raw/Kconfig
>> +++ b/drivers/mtd/nand/raw/Kconfig
>> @@ -208,6 +208,17 @@ config MTD_NAND_BRCMNAND
>>   	  originally designed for Set-Top Box but is used on various BCM7xxx,
>>   	  BCM3xxx, BCM63xxx, iProc/Cygnus and more.
>>   
>> +if MTD_NAND_BRCMNAND
>> +
>> +config MTD_NAND_BRCMNAND_BCMA
>> +	tristate "Broadcom BCMA NAND controller"
>> +	depends on BCMA_NFLASH
>> +	depends on BCMA
>> +	help
>> +	  Enables the BRCMNAND controller over BCMA on BCM47186/BCM5358 SoCs.
>> +
>> +endif # MTD_NAND_BRCMNAND
>> +
>>   config MTD_NAND_BCM47XXNFLASH
>>   	tristate "BCM4706 BCMA NAND controller"
>>   	depends on BCMA_NFLASH
>> diff --git a/drivers/mtd/nand/raw/brcmnand/Makefile b/drivers/mtd/nand/raw/brcmnand/Makefile
>> index 195b845e48b8..16dc7254200e 100644
>> --- a/drivers/mtd/nand/raw/brcmnand/Makefile
>> +++ b/drivers/mtd/nand/raw/brcmnand/Makefile
>> @@ -6,3 +6,5 @@ obj-$(CONFIG_MTD_NAND_BRCMNAND)		+= bcm63138_nand.o
>>   obj-$(CONFIG_MTD_NAND_BRCMNAND)		+= bcm6368_nand.o
>>   obj-$(CONFIG_MTD_NAND_BRCMNAND)		+= brcmstb_nand.o
>>   obj-$(CONFIG_MTD_NAND_BRCMNAND)		+= brcmnand.o
>> +
>> +obj-$(CONFIG_MTD_NAND_BRCMNAND_BCMA)	+= bcma_nand.o
>> diff --git a/drivers/mtd/nand/raw/brcmnand/bcma_nand.c b/drivers/mtd/nand/raw/brcmnand/bcma_nand.c
>> new file mode 100644
>> index 000000000000..e3be9ecf0761
>> --- /dev/null
>> +++ b/drivers/mtd/nand/raw/brcmnand/bcma_nand.c
>> @@ -0,0 +1,131 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright © 2021 Broadcom
>> + */
>> +#include <linux/bcma/bcma.h>
>> +#include <linux/bcma/bcma_driver_chipcommon.h>
>> +#include <linux/device.h>
>> +#include <linux/module.h>
>> +#include <linux/mod_devicetable.h>
>> +#include <linux/platform_device.h>
>> +
>> +#include "brcmnand.h"
>> +
>> +struct brcmnand_bcma_soc {
>> +	struct brcmnand_soc soc;
>> +	struct bcma_drv_cc *cc;
>> +};
>> +
>> +static inline bool brcmnand_bcma_needs_swapping(u32 offset)
>> +{
>> +	switch (offset) {
>> +	case BCMA_CC_NAND_SPARE_RD0:
>> +	case BCMA_CC_NAND_SPARE_RD4:
>> +	case BCMA_CC_NAND_SPARE_RD8:
>> +	case BCMA_CC_NAND_SPARE_RD12:
>> +	case BCMA_CC_NAND_SPARE_WR0:
>> +	case BCMA_CC_NAND_SPARE_WR4:
>> +	case BCMA_CC_NAND_SPARE_WR8:
>> +	case BCMA_CC_NAND_SPARE_WR12:
>> +	case BCMA_CC_NAND_DEVID:
>> +	case BCMA_CC_NAND_DEVID_X:
>> +	case BCMA_CC_NAND_SPARE_RD16:
>> +	case BCMA_CC_NAND_SPARE_RD20:
>> +	case BCMA_CC_NAND_SPARE_RD24:
>> +	case BCMA_CC_NAND_SPARE_RD28:
>> +		return true;
>> +	}
>> +
>> +	return false;
>> +}
>> +
>> +static u32 brcmnand_bcma_read_reg(struct brcmnand_soc *soc, u32 offset)
>> +{
>> +	struct brcmnand_bcma_soc *sc;
>> +	u32 val;
>> +
>> +	sc = container_of(soc, struct brcmnand_bcma_soc, soc);
>> +
>> +	/* Offset into the NAND block and deal with the flash cache separately */
>> +	if (offset == ~0)
>> +		offset = BCMA_CC_NAND_CACHE_DATA;
>> +	else
>> +		offset += BCMA_CC_NAND_REVISION;
>> +
>> +	val = bcma_cc_read32(sc->cc, offset);
>> +
>> +	/* Swap if necessary */
>> +	if (brcmnand_bcma_needs_swapping(offset))
>> +		val = be32_to_cpu(val);
>> +	return val;
>> +}
>> +
>> +static void brcmnand_bcma_write_reg(struct brcmnand_soc *soc, u32 val,
>> +				    u32 offset)
>> +{
>> +	struct brcmnand_bcma_soc *sc;
>> +
>> +	sc = container_of(soc, struct brcmnand_bcma_soc, soc);
>> +
>> +	/* Offset into the NAND block */
>> +	if (offset == ~0)
>> +		offset = BCMA_CC_NAND_CACHE_DATA;
>> +	else
>> +		offset += BCMA_CC_NAND_REVISION;
>> +
>> +	/* Swap if necessary */
>> +	if (brcmnand_bcma_needs_swapping(offset))
>> +		val = cpu_to_be32(val);
>> +
>> +	bcma_cc_write32(sc->cc, offset, val);
>> +}
>> +
>> +static struct brcmnand_io_ops brcmnand_bcma_io_ops = {
>> +	.read_reg	= brcmnand_bcma_read_reg,
>> +	.write_reg	= brcmnand_bcma_write_reg,
>> +};
>> +
>> +static void brcmnand_bcma_prepare_data_bus(struct brcmnand_soc *soc, bool prepare,
>> +					   bool is_param)
>> +{
>> +	struct brcmnand_bcma_soc *sc;
>> +
>> +	sc = container_of(soc, struct brcmnand_bcma_soc, soc);
> 
> A helper for that would be nice?

Sure, I can add a helper since we use the same construct about 3 times, 
thanks!
-- 
Florian