Re: [PATCH v2 00/11] fw_devlink improvements

Saravana Kannan
2023-01-31T01:20:47Z

On Mon, Jan 30, 2023 at 4:09 AM Maxim Kiselev <bigunclemax@gmail.com> wrote:
>
> Hi Saravana & Miquel.
>
> Sorry for the long response. I finally got access to my test device
> and tried this patch series.
>
> And unfortunately it didn't solve my issue. I'm still getting a
> hanging f1070000.ethernet dependency
> from the nvmem-cell mac@6 subnode.

Thanks for testing the series.

Btw, don't top post. It's frowned upon. Top post means your reply is
on the top before the email you are replying to. See how my first line
of reply in inline with your email I'm replying to?

>
> Here are related parts of my kernel log and device tree:
>
>
> [ 2.713302] device: 'mtd-0': device_add
> [ 2.719528] device: 'spi0': device_add
> [ 2.724180] device: 'spi0.0': device_add
> [ 2.728957] spi-nor spi0.0: mx66l51235f (65536 Kbytes)
> [ 2.735338] 7 fixed-partitions partitions found on MTD device spi0.0
> [ 2.741978] device:
> 'f1010600.spi:m25p80@0:partitions:partition@1': device_add
> [ 2.749636] Creating 7 MTD partitions on "spi0.0":
> [ 2.754564] 0x000000000000-0x000000080000 : "SPI.U_BOOT"
> [ 2.759981] device: 'mtd0': device_add
> [ 2.764323] device: 'mtd0': device_add
> [ 2.768280] device: 'mtd0ro': device_add
> [ 2.772624] 0x0000000a0000-0x0000000c0000 : "SPI.INV_INFO"
> [ 2.778218] device: 'mtd1': device_add
> [ 2.782549] device: 'mtd1': device_add
> [ 2.786582] device: 'mtd1ro': device_add
> ...
> [ 5.426625] mvneta_bm f10c0000.bm: Buffer Manager for network
> controller enabled
> [ 5.492867] platform f1070000.ethernet: error -EPROBE_DEFER:
> wait for supplier mac@6
> [ 5.528636] device: 'Fixed MDIO bus.0': device_add
> [ 5.533726] device: 'fixed-0': device_add
> [ 5.547564] device: 'f1072004.mdio-eth-mii': device_add
> [ 5.616368] device: 'f1072004.mdio-eth-mii:00': device_add
> [ 5.645127] device: 'f1072004.mdio-eth-mii:1e': device_add
> [ 5.651530] devices_kset: Moving f1070000.ethernet to end of list
> [ 5.657948] platform f1070000.ethernet: error -EPROBE_DEFER:
> wait for supplier mac@6
>
> spi@10600 {
> m25p80@0 {
> compatible = "mx66l51235l";
>
> partitions {
> compatible = "fixed-partitions";
>
> partition@0 {
> label = "SPI.U_BOOT";
> };
> partition@1 {
> compatible = "nvmem-cells";
> label = "SPI.INV_INFO";
> macaddr: mac@6 {
> reg = <0x6 0x6>;
> };
> };
> ...
> };
> };
> };
>
> enet1: ethernet@70000 {
> nvmem-cells = <&macaddr>;
> nvmem-cell-names = "mac-address";
> phy-mode = "rgmii";
> phy = <&phy0>;
> };
>
>
> Maybe I should provide some additional debug info?

I took a look at it and I think I know the issue. But it'll be good if
you can point me to the dts (not dtsi) file that corresponds to the
board you are seeing this issue on so I can double check my guess by
looking at the exact code/drivers.

The main problem/mistake is the nvmem framework is using a "struct
bus" instead of a "struct class" to keep a list of the nvmem devices.
And we can't change it now because it'd affect the sysfs paths
significantly and might break userspace ABI.

Can you try the patch at the end of this email under these
configurations and tell me which ones fail vs pass? I don't need logs
for any pass/failures.
1. On top of this series
2. Without this series
3. On top of the series but with the call to fwnode_dev_initialized() deleted?
4. Without this series, but with the call to fwnode_dev_initialized() deleted?

-Saravana

Sorry about tabs to spaces conversion. Email client issue.

diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index 321d7d63e068..23d94c0ecccf 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -752,6 +752,7 @@ static int nvmem_add_cells_from_of(struct
nvmem_device *nvmem)
struct nvmem_device *nvmem_register(const struct nvmem_config *config)
{
struct nvmem_device *nvmem;
+ struct fwnode_handle *fwnode;
int rval;

if (!config->dev)
@@ -804,9 +805,18 @@ struct nvmem_device *nvmem_register(const struct
nvmem_config *config)
nvmem->keepout = config->keepout;
nvmem->nkeepout = config->nkeepout;
if (config->of_node)
- nvmem->dev.of_node = config->of_node;
+ fwnode = of_fwnode_handle(config->of_node);
else if (!config->no_of_node)
- nvmem->dev.of_node = config->dev->of_node;
+ fwnode = of_fwnode_handle(config->dev->of_node);
+ device_set_node(&nvmem->dev, fwnode);
+
+ /*
+ * If the fwnode doesn't have another device associated with it, mark
+ * the fwnode as initialized since no driver is going to bind to the
+ * nvmem.
+ */
+ if (fwnode && !fwnode->dev)
+ fwnode_dev_initialized(fwnode, true);

switch (config->id) {