Hey Saravana,
On Thu, Jun 15, 2023 at 12:17:08PM -0700, Saravana Kannan wrote:
> On Tue, Jun 13, 2023 at 8:35 AM Anup Patel <apatel@xxxxxxxxxxxxxxxx> wrote:
btw, please try to delete the 100s of lines of unrelated context when
replying
> > +static int __init aplic_dt_init(struct device_node *node,
> > + struct device_node *parent)
> > +{
> > + /*
> > + * The APLIC platform driver needs to be probed early
> > + * so for device tree:
> > + *
> > + * 1) Set the FWNODE_FLAG_BEST_EFFORT flag in fwnode which
> > + * provides a hint to the device driver core to probe the
> > + * platform driver early.
> > + * 2) Clear the OF_POPULATED flag in device_node because
> > + * of_irq_init() sets it which prevents creation of
> > + * platform device.
> > + */
> > + node->fwnode.flags |= FWNODE_FLAG_BEST_EFFORT;
>
> NACK. You are blindly plastering flags without trying to understand
> the real issue and fixing this correctly.
>
> > + of_node_clear_flag(node, OF_POPULATED);
> > + return 0;
> > +}
> > +IRQCHIP_DECLARE(riscv_aplic, "riscv,aplic", aplic_dt_init);
>
> This macro pretty much skips the entire driver core framework to probe
> and calls init and you are supposed to initialize the device when the
> init function is called.
>
> If you want your device/driver to follow the proper platform driver
> path (which is recommended), then you need to use the
> IRQCHIP_PLATFORM_DRIVER_BEGIN() and related macros. Grep for plenty of examples.
>
> I offered to help you debug this issue and I asked for a dts file that
> corresponds to a board you are testing this on and seeing an issue.
There isn't a dts file for this because there's no publicly available
hardware that actually has an APLIC. Maybe Ventana have pre-production
silicon that has it, but otherwise it's a QEMU job.
Cheers,
Conor.
> But you haven't answered my question [1] and are pointing to some
> random commit and blaming it. That commit has no impact on any
> existing devices/drivers.
>
> Hi Marc,
>
> Please consider this patch Nacked as long as FWNODE_FLAG_BEST_EFFORT
> is used or until Anup actually works with us to debug the real issue.
>
> -Saravana
> [1] - https://lore.kernel.org/lkml/CAAhSdy2p6K70fc2yZLPdVGqEq61Y8F7WVT2J8st5mQrzBi4WHg@xxxxxxxxxxxxxx/
Attachment:
signature.asc
Description: PGP signature
