Re: [PATCHv2 1/3] misc: clean up bmp085 driver

[Arnd Bergmann]

On Monday 12 March 2012, Eric Andersson wrote:
> > > diff --git a/include/linux/i2c/bmp085.h b/include/linux/i2c/bmp085.h
> > > new file mode 100644
> > > index 0000000..e6fc752
> > > --- /dev/null
> > > +++ b/include/linux/i2c/bmp085.h
> > 
> > Since this file only adds platform_data, I think it should go into
> > include/linux/platform_data/, not include/linux/i2c, and it should
> > be in the same patch as the change to use the platform data when you
> > split that out.
> > 
> > Also, which platforms are actually using this driver? I could
> > not find any platform that defines a bmp085 platform_device. If this
> > is for new ARM platforms, I would rather not add platform_data at
> > all because those platforms will have to use device tree properties
> > rather than platform_data to pass initialization data.
> 
> Platform device? This is a pressure sensor connected as a peripheral
> device to the i2c bus. I don't think you will find any platform that
> uses this "by default" in the tree.
> 
> Of course I can add of_get_property() calls, but what is actually the
> long-term plan here for peripheral devices? Should they all abandon their
> platform data in favor of devicetree? 

Yes, that is definitely the plan. The current method does not scale,
because right now you have to modify the kernel sources any time you
have a slightly different combination of devices in the system, which
especially in cases of user-pluggable i2c devices is not a good
design.

> How should the callbacks in platform data be handled, in my case
> pdata->init_hw()?

This depends a lot on what the individual callback does. In my experience,
90% of these callbacks can and should just get removed because they
are not necessary and the same functionality can be expressed by generalizing
the setup procedure, e.g. by calling into the regulator, gpio, clock
or pinmux frameworks from the device probe() function.

In the few cases where this is not possible, some of the approaches
I can think of (again, case by case) include:

* educate the hardware designers so they do things in a more maintainable
  way in the future, hope that the current design goes away.

* add a special case in the driver using something like
  if (machine_is_compatible("something,weird"))
	do_weird_setup(dev);

* use auxdata for the machine to supply a platform_data structure in
  addition to the device tree.

* Don't support the board in the mainline kernel, but require the
  user to keep using a private patch (this is the status quo, because
  right now they need a private patch to add the board file).

	Arnd

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/