On Wed, Aug 03, 2022 at 01:45:04PM +0200, Andy Shevchenko wrote:

On Wed, Aug 3, 2022 at 7:48 AM Colin Foster
[off-list ref] wrote:

quoted

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.

...

quoted

+#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>

...

quoted

+struct regmap *ocelot_spi_init_regmap(struct device *dev, const struct resource *res)
+{
+       struct regmap_config regmap_config;
+
+       memcpy(&regmap_config, &ocelot_spi_regmap_config, sizeof(regmap_config));
+
+       regmap_config.name = res->name;

quoted

+       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

quoted

+       regmap_config.reg_base = res->start;
+
+       return devm_regmap_init(dev, &ocelot_spi_regmap_bus, dev, &regmap_config);
+}

-- 
With Best Regards,
Andy Shevchenko