On Wed, Aug 03, 2022 at 01:45:04PM +0200, Andy Shevchenko wrote:
> On Wed, Aug 3, 2022 at 7:48 AM Colin Foster
> <colin.foster@xxxxxxxxxxxxxxxx> wrote:
> >
> > The VSC7512 is a networking chip that contains several peripherals. Many of
> > these peripherals are currently supported by the VSC7513 and VSC7514 chips,
> > but those run on an internal CPU. The VSC7512 lacks this CPU, and must be
> > controlled externally.
> >
> > Utilize the existing drivers by referencing the chip as an MFD. Add support
> > for the two MDIO buses, the internal phys, pinctrl, and serial GPIO.
>
>
> ...
>
> > +#include <asm/byteorder.h>
>
> Not sure I see the user of this header.
Interesting. And I think you uncovered one more issue.
I'd used byteorder at one time to modify the SPI payload (addr is always
big-endian, payload should always be native).
When I migrated to using the spi_bus_read() instead of spi_reg_read(),
this became handled much more elegantly in regmap itself. So this isn't
needed anymore.
I also have byteorder in include/linux/mfd/ocelot.h, where it also isn't
needed. It is checking:
#ifdef __LITTLE_ENDIAN
#define OCELOT_SPI_BYTE_ORDER OCELOT_SPI_BYTE_ORDER_LE
#else
#define OCELOT_SPI_BYTE_ORDER OCELOT_SPI_BYTE_ORDER_BE
#endif
That file should be replaced with
#include <linux/kconfig.h>
>
> ...
>
> > +struct regmap *ocelot_spi_init_regmap(struct device *dev, const struct resource *res)
> > +{
> > + struct regmap_config regmap_config;
> > +
> > + memcpy(®map_config, &ocelot_spi_regmap_config, sizeof(regmap_config));
> > +
> > + regmap_config.name = res->name;
>
> > + regmap_config.max_register = res->end - res->start;
>
> Hmm... First of all, resource_size() is for that (with - 1 to the
> result). But don't you need to use stride in the calculations?
DEFINE_RES_NAMED populates the resource .end with (_start) + (_size) - 1
so I don't think resource_size is correct to use here.
reg_stride gets handled at the top of regmap_read(), so I don't think
that's really needed either.
For reference:
#define VSC7512_DEVCPU_ORG_RES_START 0x71000000
#define VSC7512_DEVCPU_ORG_RES_SIZE 0x38
# pwd
/sys/kernel/debug/regmap/spi0.0-devcpu_org
# cat range
0-34
# cat registers
00: 00000000
04: 02000001
08: 00000001
0c: 00000000
10: 00000fff
14: 00000000
18: 00000000
1c: 00000000
20: 00000000
24: 00000000
28: 00000001
2c: 00000004
30: 00000001
34: 00000004
>
> > + regmap_config.reg_base = res->start;
> > +
> > + return devm_regmap_init(dev, &ocelot_spi_regmap_bus, dev, ®map_config);
> > +}
>
> --
> With Best Regards,
> Andy Shevchenko
