mbox series

[v2,0/2] of: property: Add fw_devlink support for more props

Message ID 20210121225712.1118239-1-saravanak@google.com
Headers show
Series of: property: Add fw_devlink support for more props | expand

Message

Saravana Kannan Jan. 21, 2021, 10:57 p.m. UTC
Sending again because I messed up the To/Cc for the coverletter.

This series combines two patches [1] [2] that'd conflict.

Greg,

Can you please pull this into driver-core-next?

-Saravana

[1] - https://lore.kernel.org/lkml/20210115210159.3090203-1-saravanak@google.com/
[2] - https://lore.kernel.org/lkml/20201218210750.3455872-1-saravanak@google.com/

Individual -> Series:
Patch 1/2: Addressed Geert's gpio-hog problem with gpio[s] property
Patch 2/2: Switched to using of_irq_find_parent()

v1 -> v2:
Patch 1/2: Added the Reviewed-by tags
Patch 2/2: Updated commit text and comments. Added Reviewed-by tags.

Saravana Kannan (2):
  of: property: Add fw_devlink support for "gpio" and "gpios" binding
  of: property: Add fw_devlink support for interrupts

 drivers/of/property.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

Comments

Greg Kroah-Hartman Jan. 26, 2021, 8:22 a.m. UTC | #1
On Thu, Jan 21, 2021 at 02:57:10PM -0800, Saravana Kannan wrote:
> Sending again because I messed up the To/Cc for the coverletter.
> 
> This series combines two patches [1] [2] that'd conflict.
> 
> Greg,
> 
> Can you please pull this into driver-core-next?

Now queued up, thanks.

greg k-h
Saravana Kannan Jan. 31, 2021, 9:05 p.m. UTC | #2
On Sun, Jan 31, 2021 at 8:38 AM Martin Kaiser <martin@kaiser.cx> wrote:
>
> Dear all,
>
> Thus wrote Saravana Kannan (saravanak@google.com):
>
> > Sending again because I messed up the To/Cc for the coverletter.
>
> > This series combines two patches [1] [2] that'd conflict.
>
> > Greg,
>
> > Can you please pull this into driver-core-next?
>
> > -Saravana
>
> > [1] - https://lore.kernel.org/lkml/20210115210159.3090203-1-saravanak@google.com/
> > [2] - https://lore.kernel.org/lkml/20201218210750.3455872-1-saravanak@google.com/
>
> I'm running linux-next on my hardware which is based on the imx258
> chipset by Freescale/NXP.
>
> When those two patches appeared in linux-next, my system would not boot
> any more. It was stuck right after
>
> Uncompressing Linux... done, booting the kernel.
>
> Reverting the irq-patch made the system boot again. Still, a number of
> devices like usb or nand flash controller are not found any more.
> If I revert the gpio patch as well, all devices are available again.
>
> My system's device tree is based on arch/arm/boot/dts/imx25.dtsi with
> very few adaptations for my board.
>
> I tried to play around with the new parse_interrupts() function to
> figure out which device causes the boot failure. If I skip the following
> device, I can boot again:
>
> -       return of_irq_find_parent(np);
> +       np_ret = of_irq_find_parent(np);
> +       if (!strcmp(np->full_name, "serial@50008000")) {
> +           printk(KERN_ERR "skip serial@50008000\n");
> +           return NULL;
> +       }
> +      return np_ret;
>
> This is uart4 of the imx258 chip, which I use as my serial console. The
> imx25.dtsi device tree seems ok, we find an interrupt parent. The
> problem must be in the code that processes the result of
> parse_interrupts().
>
> I tried to boot the unmodified code with qemu, simulating the imx25-pdk
> device. This wouldn't boot either.
>
> Does this ring any bells with anyone?

This series [1] has a high chance of fixing it for you if
CONFIG_MODULES is disabled in your set up. Can you give it a shot?

The real problem is that arch/arm/mach-imx/avic.c doesn't set the
OF_POPULATED flag for the "fsl,avic" node. fw_devlink uses this
information to know that this device node will never have a struct
device created for it. The proper way to do this for root IRQCHIP
nodes is to use IRQCHIP_DECLARE(). I Cc'ed you on a clean up patch for
IMX [2], can you please give [2] a shot *without* [1] and with
CONFIG_MODULES enabled? Things should boot properly with this
combination too.

Btw, for future reference, you can try enabling the logs in
device_links_check_suppliers() to see what devices are being blocked
on what supplier nodes.

[1] - https://lore.kernel.org/lkml/20210130040344.2807439-1-saravanak@google.com/
[2] - https://lore.kernel.org/lkml/20210131205654.3379661-1-saravanak@google.com/T/#u

Thanks,
Saravana
Martin Kaiser Feb. 1, 2021, 10:52 a.m. UTC | #3
Hi Saravana,

Thus wrote Saravana Kannan (saravanak@google.com):

> This series [1] has a high chance of fixing it for you if
> CONFIG_MODULES is disabled in your set up. Can you give it a shot?

sure. This fixes things for me if CONFIG_MODULES is disabled. Booting is
still stuck if modules are enabled.

> The real problem is that arch/arm/mach-imx/avic.c doesn't set the
> OF_POPULATED flag for the "fsl,avic" node. fw_devlink uses this
> information to know that this device node will never have a struct
> device created for it. The proper way to do this for root IRQCHIP
> nodes is to use IRQCHIP_DECLARE(). I Cc'ed you on a clean up patch for
> IMX [2], can you please give [2] a shot *without* [1] and with
> CONFIG_MODULES enabled? Things should boot properly with this
> combination too.

This works as well.

Thanks,
Martin
Saravana Kannan Feb. 1, 2021, 8:04 p.m. UTC | #4
On Mon, Feb 1, 2021 at 2:52 AM Martin Kaiser <martin@kaiser.cx> wrote:
>
> Hi Saravana,
>
> Thus wrote Saravana Kannan (saravanak@google.com):
>
> > This series [1] has a high chance of fixing it for you if
> > CONFIG_MODULES is disabled in your set up. Can you give it a shot?
>
> sure. This fixes things for me if CONFIG_MODULES is disabled. Booting is
> still stuck if modules are enabled.
>
> > The real problem is that arch/arm/mach-imx/avic.c doesn't set the
> > OF_POPULATED flag for the "fsl,avic" node. fw_devlink uses this
> > information to know that this device node will never have a struct
> > device created for it. The proper way to do this for root IRQCHIP
> > nodes is to use IRQCHIP_DECLARE(). I Cc'ed you on a clean up patch for
> > IMX [2], can you please give [2] a shot *without* [1] and with
> > CONFIG_MODULES enabled? Things should boot properly with this
> > combination too.
>
> This works as well.

Thanks for testing both. Mind giving Tested-by for [1] too?

-Saravana