On Sun, Feb 5, 2023 at 5:32 PM Saravana Kannan <saravanak@xxxxxxxxxx> wrote: > > On Fri, Feb 3, 2023 at 1:39 AM Maxim Kiselev <bigunclemax@xxxxxxxxx> wrote: > > > > пт, 3 февр. 2023 г. в 09:07, Saravana Kannan <saravanak@xxxxxxxxxx>: > > > > > > On Thu, Feb 2, 2023 at 9:36 AM Maxim Kiselev <bigunclemax@xxxxxxxxx> wrote: > > > > > > > > Hi Saravana, > > > > > > > > > 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 > > > > > > > > I did these tests and here is the results: > > > > > > Did you hand edit the In-Reply-To: in the header? Because in the > > > thread you are reply to the wrong email, but the context in your email > > > seems to be from the right email. > > > > > > For example, see how your reply isn't under the email you are replying > > > to in this thread overview: > > > https://lore.kernel.org/lkml/20230127001141.407071-1-saravanak@xxxxxxxxxx/#r > > > > > > > 1. On top of this series - Not works > > > > 2. Without this series - Works > > > > 3. On top of the series with the fwnode_dev_initialized() deleted - Not works > > > > 4. Without this series, with the fwnode_dev_initialized() deleted - Works > > > > > > > > So your nvmem/core.c patch helps only when it is applied without the series. > > > > But despite the fact that this helps to avoid getting stuck at probing > > > > my ethernet device, there is still regression. > > > > > > > > When the ethernet module is loaded it takes a lot of time to drop dependency > > > > from the nvmem-cell with mac address. > > > > > > > > Please look at the kernel logs below. > > > > > > The kernel logs below really aren't that useful for me in their > > > current state. See more below. > > > > > > ---8<---- <snip> --->8---- > > > > > > > P.S. Your nvmem patch definitely helps to avoid a device probe stuck > > > > but look like it is not best way to solve a problem which we discussed > > > > in the MTD thread. > > > > > > > > P.P.S. Also I don't know why your nvmem-cell patch doesn't help when it was > > > > applied on top of this series. Maybe I missed something. > > > > > > Yeah, I'm not too sure if the test was done correctly. You also didn't > > > answer my question about the dts from my earlier email. > > > https://lore.kernel.org/lkml/CAGETcx8FpmbaRm2CCwqt3BRBpgbogwP5gNB+iA5OEtuxWVTNLA@xxxxxxxxxxxxxx/#t > > > > > > So, can you please retest config 1 with all pr_debug and dev_dbg in > > > drivers/core/base.c changed to the _info variants? And then share the > > > kernel log from the beginning of boot? Maybe attach it to the email so > > > it doesn't get word wrapped by your email client. And please point me > > > to the .dts that corresponds to your board. Without that, I can't > > > debug much. > > > > > > Thanks, > > > Saravana > > > > > Did you hand edit the In-Reply-To: in the header? Because in the > > > thread you are reply to the wrong email, but the context in your email > > > seems to be from the right email. > > > > Sorry for that, it seems like I accidently deleted it. > > > > > So, can you please retest config 1 with all pr_debug and dev_dbg in > > > drivers/core/base.c changed to the _info variants? And then share the > > > kernel log from the beginning of boot? Maybe attach it to the email so > > > it doesn't get word wrapped by your email client. And please point me > > > to the .dts that corresponds to your board. Without that, I can't > > > debug much. > > > > Ok, I retested config 1 with all _debug logs changed to the _info. I > > added the kernel log and the dts file to the attachment of this email. > > Ah, so your device is not supported/present upstream? Even though it's > not upstream, I'll help fix this because it should fix what I believe > are unreported issues in upstream. > > Ok I know why configs 1 - 4 behaved the way they did and why my test > patch didn't help. > > After staring at mtd/nvmem code for a few hours I think mtd/nvmem > interaction is kind of a mess. mtd core creates "partition" platform > devices (including for nvmem-cells) that are probed by drivers in > drivers/nvmem. However, there's no driver for "nvmem-cells" partition > platform device. However, the nvmem core creates nvmem_device when > nvmem_register() is called by MTD or these partition platform devices > created by MTD. But these nvmem_devices are added to a nvmem_bus but > the bus has no means to even register a driver (it should really be a > nvmem_class and not nvmem_bus). And the nvmem_device sometimes points > to the DT node of the MTD device or sometimes the partition platform > devices or maybe no DT node at all. > > So it's a mess of multiple devices pointing to the same DT node with > no clear way to identify which ones will point to a DT node and which > ones will probe and which ones won't. In the future, we shouldn't > allow adding new compatible strings for partitions for which we don't > plan on adding nvmem drivers. > > Can you give the patch at the end of the email a shot? It should fix > the issue with this series and without this series. It just avoids > this whole mess by not creating useless platform device for > nvmem-cells compatible DT nodes. Actually, without this series, the patch below will need an additional line of code inside the if block: fwnode_dev_initialized(of_fwnode_handle(child), true); -Saravana > > Thanks, > Saravana > > diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c > index d442fa94c872..88a213f4d651 100644 > --- a/drivers/mtd/mtdpart.c > +++ b/drivers/mtd/mtdpart.c > @@ -577,6 +577,7 @@ static int mtd_part_of_parse(struct mtd_info *master, > { > struct mtd_part_parser *parser; > struct device_node *np; > + struct device_node *child; > struct property *prop; > struct device *dev; > const char *compat; > @@ -594,6 +595,10 @@ static int mtd_part_of_parse(struct mtd_info *master, > else > np = of_get_child_by_name(np, "partitions"); > > + for_each_child_of_node(np, child) > + if (of_device_is_compatible(child, "nvmem-cells")) > + of_node_set_flag(child, OF_POPULATED); > + > of_property_for_each_string(np, "compatible", prop, compat) { > parser = mtd_part_get_compatible_parser(compat); > if (!parser)
