Hi Geert Uytterhoeven,
Thanks for the feedback.
> Subject: Re: [PATCH 2/2 RESEND] power: supply: sbs-battery: Convert enum-
> >pointer for data in the match tables
>
> Hi Biju,
>
> On Sun, Aug 20, 2023 at 7:12 PM Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> wrote:
> > Convert enum->pointer for data in the match tables, so that
> > device_get_match_data() can do match against OF/ACPI/I2C tables, once
> > i2c bus type match support added to it and it returns NULL for non-match.
> >
> > Therefore it is better to convert enum->pointer for data match and
> > extend match support for both ID and OF tables using
> > i2c_get_match_data() by adding struct sbs_data with flags variable and
> > replacing flags->data in struct sbs_info.
> >
> > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
>
> Thanks for your patch!
>
> > --- a/drivers/power/supply/sbs-battery.c
> > +++ b/drivers/power/supply/sbs-battery.c
> > @@ -201,6 +201,10 @@ static const enum power_supply_property
> > string_properties[] = {
> >
> > #define NR_STRING_BUFFERS ARRAY_SIZE(string_properties)
> >
> > +struct sbs_data {
> > + u32 flags;
> > +};
>
> Unless you plan to add more members to struct sbs_data, I see no point in
> this patch: it only increases kernel size.
>
> The various "data" members in <foo>_id structures are intended to contain
> either a pointer or a single integral value.
The match data value for sbs_battery is 0. Here the API returns
NULL for a non-match. That is the reason it is converted to pointer.
So, we cannot differentiate actual matched data and error in this case.
Not sure, cases like this to be split into individual matches like
of_device_get_match(), acpi_match and ID look up instead so that majority of the drivers using uniform OF/ACPI/ID tables and pointers/enums with NoN zero are getting benefitted from proposed device_get_match_data()??
Cheers,
Biju
