Re: [PATCH v6 09/10] power: supply: Support ROHM bd99954 charger

[Vaittinen, Matti]

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