Message-ID: <20110509133542.GF3225@opensource.wolfsonmicro.com>
Date:	Mon, 9 May 2011 15:35:43 +0200
From:	Mark Brown <broonie@...nsource.wolfsonmicro.com>
To:	Felipe Balbi <balbi@...com>
Cc:	Liam Girdwood <lrg@...nel.org>,
	Graeme Gregory <gg@...mlogic.co.uk>,
	linux-omap@...r.kernel.org, linux-kernel@...r.kernel.org,
	sameo@...ux.intel.com
Subject: Re: [PATCH 4/4] USB: TWL6025 allow different regulator name

On Mon, May 09, 2011 at 04:07:07PM +0300, Felipe Balbi wrote:

> The thing is. We already had problems in the past with the Clock FW
> because drivers were passing clock names on platform_data and I really
> want to avoid the same on regulators. We need something clever.

Right, which is why you should *never* do that.  Any time I see anyone
requesting a regulator based on anything other than a fixed string in
the driver and the struct device for the consumer I push back very hard,
and so should everyone else.

> We pass in the data of how regulators are wired at the board level and
> drivers would regulator_get(my_dev->dev, "whatever fancy name I want as
> it doesn't matter at all");

No, not at all.  Consumer drivers should be hard coding the strings they
request and the strings used should correspond to the pin names used on
the device.

> If a driver has >1 regulator, you can distinguish them by which
> functionality they provide, no matter the name, they will be connected
> to a particular ball on the IC package which has a certain definition

Having more than one supply is the common case for devices; relatively
few devices have a single supply.

> on the TRM. In other words, we should match on the functionality they
> provide, not on the name.

As Liam said this will just make the situation worse.  Users will have
to figure out what the names the driver authors assigned to the various
device supplies map onto in the physical system via an additional layer
of indirection which will at best be written down in comments in the
driver.

> Let's try to thing some 5 - 6 years ahead. With the current trend, we
> will be working on support for the PMIC on OMAP10, imagine if each one
> of those revisions decide to change the name of the regulator, because
> this new HW engineer working on the PMIC design doesn't like the old
> name. We will have X regulator pointers and X regulator name pointers in
> our platform_data. It will be really cumbersome and prone to errors if
> people continue on copy&pasting old code to "generate" a new driver.

Here you're apparently talking about something different - you're
talking about how we pass in the regulator init data for the regulators
supplied by the chip.  The patch being discussed changes the consumer
side for USB, not the regulator driver side.  The naming on the driver
side is a particular problem for TWL devices since unlike most PMICs the
regulators aren't simply numbered but are instead assigned individual
names so you can't just use a big array indexed by number.

> What I was expecting to see, was a mechanism where we define how those
> things are wired (it doesn't matter if the data uses DeviceTree or what)
> at the HW level and we ask the framework to give us the regulator
> connected to device X which provides a certain functionality. Just like
> clkdev has managed to get close to. Currently drivers will:

This is preceisely what is happening.  A well written consumer driver
asks the regulator core for the regulator supplying a given supply, with
the supplies named in the same way as they are named in the datasheet.

You appear to be arging strongly for us to adopt the model which is
already in use.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists