On Tue, 2020-03-24 at 13:56 +0200, andriy.shevchenko@xxxxxxxxxxxxxxx
wrote:
> On Tue, Mar 24, 2020 at 10:53:09AM +0000, Vaittinen, Matti wrote:
> > On Tue, 2020-03-24 at 11:50 +0200, Andy Shevchenko wrote:
> > > On Tue, Mar 24, 2020 at 10:32:19AM +0200, Matti Vaittinen wrote:
>
> ...
>
> > > > + for (i = ffs(tmp); i; i = ffs(tmp)) {
> > >
> > > NIH of for_each_set_bit().
> >
> > What does the NIH stand for?
>
> Not Invented Here syndrome :-)
Ah. I definitely plead guilty for that in general. Or even NIBM (Not
Invented By Me) :] But at this time for_each_set_bit() just didn't come
to my mind - and I used ffs() - even though that's not invented by me
either ;)
I just modified the driver to use for_each_set_bit() and you were
correct. Result is _much_ prettier. Thanks! That'll be fixed in v7.
> ...
>
> > > > + if (!dev->platform_data) {
> > >
> > > dev_get_platdata()
> > >
> > > > + ret = bd9995x_fw_probe(bd);
> > > > + if (ret < 0) {
> > > > + dev_err(dev, "Cannot read device
> > > > properties.\n");
> > > > + return ret;
> > > > + }
> > > > + } else {
> > > > + return -ENODEV;
> > >
> > > So, existing platform data leads to an error?!
> >
> > Yes. As currently we only use DT. If someone needs platdata they
> > need
> > to improve the driver
>
> I think the idea to avoid platform data in new code as much as
> possible.
> And it's unusual to have somebody to use this driver with
> platform_data set.
> Why not simple ignore it?
Because if someone _is_ using platform data here (and we still provide
this mechanism) - then we should inform him that he's doing something
which is not correct.
Best Regards
Matti
