Re: [PATCH v2 21/24] rave-sp: Support for variants on modalias drivers

[Ricardo Ribalda Delgado]

Hello


On Mon, Jun 11, 2018 at 3:38 PM Marcel Holtmann <marcel@xxxxxxxxxxxx> wrote:
>
> Hi Andy,
>
> >> Rave-sp behaves differently based on the device variant.
> >
> >
> >>        sp->variant = of_device_get_match_data(dev);
> >> +       if (!sp->variant) {
> >> +               id = serdev_match_id(rave_sp_serdev_id, serdev);
> >
> > I think you may leave the ID table where it is in the code and use link.
> >
> >> +               if (id)
> >> +                       sp->variant = (const struct rave_sp_variant *)
> >> +                                                       id->driver_data;
> >> +       }
> >
> > Perhaps a helper like it's done for ACPI / OF cases?
> >
> > [device_get_match_data() -> ]
> >  of_fwnode_device_get_match_data()
> >  acpi_fwnode_device_get_match_data()
>
> something like that and frankly this is trying to hard. This driver is currently really DT specific and can be fixed up later. It is causing a lot of noise in this patch series. I would really urge to focus on the core changes get prominent drivers support. I think hci_bcm.c is a good example since a) we need to fix up Edison and b) new ACPI based tablets might be able to allow for easy testing.

I can try to implement something like andy proposes, but if it turns
out too complicated we can remove this driver from the series.

Thanks!

>
> Regards
>
> Marcel
>


-- 
Ricardo Ribalda