Re: [RFC PATCH 1/5] drivers : introduce device_path api

2012-12-11 Thread Jassi Brar
On Mon, Dec 10, 2012 at 3:18 PM, Roger Quadros rog...@ti.com wrote:
 On 12/06/2012 04:34 PM, Jassi Brar wrote:

 Yes, this is where we can think of a generic PHY driver to make sure thy
 PHY has necessary resources enabled. In the Panda case it would be the
 PHY clock and RESET gpio.

 I wonder if ehci-omap should assume, besides regulator, a clock might
 also be need to setup for the transceiver chip.
 Maybe usbhs_bdata in board-omap4panda.c should have
 .reset_gpio_port[0]  = GPIO_HUB_NRESET ?


 Just like the regulator, reset_gpio_port has nothing to do with OMAP
 EHCI. So we would want to get rid of that too from the OMAP USB driver.

Looking at the code I realized we already book resources only for
populated ports. Saving power from LAN9514 chip would come from a
separate solution. So, for now when our transceiver, USB3320, has
simple hardwired configuration probably just platform_data/DT would
do. When we come across some programmable transceiver (like USB3503
over i2c), that might warrant a separate PHY driver. Still I don't
think we could have a 'generic' phy driver.


 The LAN95xx chip still needs to be powered up and the PHY driver isn't
 the right place for that (though it could be done there as a hack). The
 closest we can get to emulating right USB behavior is to map the
 SET/CLEAR PORT_FEATURE of the root hub port to the regulator that powers
 the LAN95xx.

 This way, we still don't fall in the dynamically probed space, so we
 could still provide the regulator information to the ehci hub via
 platform data and handle the regulator in ehci_hub_control
 (Set/ClearPortFeature). I'm looking at this as a software workaround for
 all platforms not implementing the EHCI set port power feature
 correctly. The same could be repeated for other HCDs as well if required.

 IMHO it's not a matter of platforms not implementing EHCI set port
 power feature correctly, we should think about onboard devices
 connected to onboard non-root hubs. Setups like LAN9514 on Panda (HSIC
 too ?) that don't run on VBUS of USB, would like their local supply to
 be treated as if it came from the parent hub's port  i.e, tie together
 the USB's set port power control with their OOB regulated power supply
 (U9 on PandaBoard).


 On Pandaboards we are still talking about root hub ports.

 Do you know any of such platforms which power their USB devices out of
 band for _non_ root hub ports?

I don't know of any. But I do believe we shouldn't discount that scenario.
IIANW lan9514 doesn't take in VBUS because it wants to avoid 5V-3.3V
regulating. What if someone designs an omap4 platform with 3
high-speed devices and the same concern in mind ?

 Assuming they do exist, the only solution is to match platform data for
 dynamically probed devices and deal with the regulators in the hub/port
 driver. Something like Andy already proposed.

Yes, but I doubt if that is the only implementation.
One USB specific solution could be to abstract out OOB port power
control in, say, port-power.c which constructs a regulator topology
mapped directly onto onboard devices', from a generic DT binding,
platform_data, ACPI whatever. And then catch any
set/clear_port_feature(POWER) call to enable/disable the regulator
corresponding to that port. Where each regulator could be a
board-specific virtual one, that does all that is necessary (like
clock/reg hierarchy setup) to power up the device.

Regards.
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH 1/5] drivers : introduce device_path api

2012-12-11 Thread Roger Quadros
On 12/11/2012 11:12 AM, Jassi Brar wrote:
 On Mon, Dec 10, 2012 at 3:18 PM, Roger Quadros rog...@ti.com wrote:
 On 12/06/2012 04:34 PM, Jassi Brar wrote:

 Yes, this is where we can think of a generic PHY driver to make sure thy
 PHY has necessary resources enabled. In the Panda case it would be the
 PHY clock and RESET gpio.

 I wonder if ehci-omap should assume, besides regulator, a clock might
 also be need to setup for the transceiver chip.
 Maybe usbhs_bdata in board-omap4panda.c should have
 .reset_gpio_port[0]  = GPIO_HUB_NRESET ?


 Just like the regulator, reset_gpio_port has nothing to do with OMAP
 EHCI. So we would want to get rid of that too from the OMAP USB driver.

 Looking at the code I realized we already book resources only for
 populated ports. Saving power from LAN9514 chip would come from a
 separate solution. So, for now when our transceiver, USB3320, has
 simple hardwired configuration probably just platform_data/DT would
 do. When we come across some programmable transceiver (like USB3503
 over i2c), that might warrant a separate PHY driver. Still I don't
 think we could have a 'generic' phy driver.
 
Yes I2C based Phys might need a different driver. At least we could have
a generic PHY driver for all ULPI based phys. (NOTE: the USB3320 is also
configurable and can work in OTG mode)
ULPI spec has defined a standard register map which could be used by the
generic driver. As far as OMAP is concerned, the ULPI registers are
accessed most of the time internally by the USB Host IP. We might only
need to access them from the driver to cover some erratas.


 The LAN95xx chip still needs to be powered up and the PHY driver isn't
 the right place for that (though it could be done there as a hack). The
 closest we can get to emulating right USB behavior is to map the
 SET/CLEAR PORT_FEATURE of the root hub port to the regulator that powers
 the LAN95xx.

 This way, we still don't fall in the dynamically probed space, so we
 could still provide the regulator information to the ehci hub via
 platform data and handle the regulator in ehci_hub_control
 (Set/ClearPortFeature). I'm looking at this as a software workaround for
 all platforms not implementing the EHCI set port power feature
 correctly. The same could be repeated for other HCDs as well if required.

 IMHO it's not a matter of platforms not implementing EHCI set port
 power feature correctly, we should think about onboard devices
 connected to onboard non-root hubs. Setups like LAN9514 on Panda (HSIC
 too ?) that don't run on VBUS of USB, would like their local supply to
 be treated as if it came from the parent hub's port  i.e, tie together
 the USB's set port power control with their OOB regulated power supply
 (U9 on PandaBoard).


 On Pandaboards we are still talking about root hub ports.

 Do you know any of such platforms which power their USB devices out of
 band for _non_ root hub ports?

 I don't know of any. But I do believe we shouldn't discount that scenario.
 IIANW lan9514 doesn't take in VBUS because it wants to avoid 5V-3.3V
 regulating. What if someone designs an omap4 platform with 3
 high-speed devices and the same concern in mind ?
 
 Assuming they do exist, the only solution is to match platform data for
 dynamically probed devices and deal with the regulators in the hub/port
 driver. Something like Andy already proposed.

 Yes, but I doubt if that is the only implementation.
 One USB specific solution could be to abstract out OOB port power
 control in, say, port-power.c which constructs a regulator topology
 mapped directly onto onboard devices', from a generic DT binding,
 platform_data, ACPI whatever. And then catch any
 set/clear_port_feature(POWER) call to enable/disable the regulator
 corresponding to that port. Where each regulator could be a
 board-specific virtual one, that does all that is necessary (like
 clock/reg hierarchy setup) to power up the device.
 

Yes. This is what I too was suggesting earlier. The big question here is
how to match the regulator to the hub port. One way was matching the
device paths.

regards,
-roger

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH 1/5] drivers : introduce device_path api

2012-12-11 Thread Felipe Balbi
On Tue, Dec 11, 2012 at 12:01:57PM +0200, Roger Quadros wrote:
 On 12/11/2012 11:12 AM, Jassi Brar wrote:
  On Mon, Dec 10, 2012 at 3:18 PM, Roger Quadros rog...@ti.com wrote:
  On 12/06/2012 04:34 PM, Jassi Brar wrote:
 
  Yes, this is where we can think of a generic PHY driver to make sure thy
  PHY has necessary resources enabled. In the Panda case it would be the
  PHY clock and RESET gpio.
 
  I wonder if ehci-omap should assume, besides regulator, a clock might
  also be need to setup for the transceiver chip.
  Maybe usbhs_bdata in board-omap4panda.c should have
  .reset_gpio_port[0]  = GPIO_HUB_NRESET ?
 
 
  Just like the regulator, reset_gpio_port has nothing to do with OMAP
  EHCI. So we would want to get rid of that too from the OMAP USB driver.
 
  Looking at the code I realized we already book resources only for
  populated ports. Saving power from LAN9514 chip would come from a
  separate solution. So, for now when our transceiver, USB3320, has
  simple hardwired configuration probably just platform_data/DT would
  do. When we come across some programmable transceiver (like USB3503
  over i2c), that might warrant a separate PHY driver. Still I don't
  think we could have a 'generic' phy driver.
  
 Yes I2C based Phys might need a different driver. At least we could have
 a generic PHY driver for all ULPI based phys. (NOTE: the USB3320 is also
 configurable and can work in OTG mode)

we already do, that's what nop-xceiv is for.

-- 
balbi


signature.asc
Description: Digital signature


Re: [RFC PATCH 1/5] drivers : introduce device_path api

2012-12-10 Thread Felipe Balbi
Hi,

On Mon, Dec 10, 2012 at 11:48:51AM +0200, Roger Quadros wrote:
   (or in appropriate order if the above isn't)
  That way insmod/rmmod'ing the USB3320C driver would power-up/down the
  whole subsystem.
 
  Yes, this is where we can think of a generic PHY driver to make sure thy
  PHY has necessary resources enabled. In the Panda case it would be the
  PHY clock and RESET gpio.
 
  I wonder if ehci-omap should assume, besides regulator, a clock might
  also be need to setup for the transceiver chip.
  Maybe usbhs_bdata in board-omap4panda.c should have
  .reset_gpio_port[0]  = GPIO_HUB_NRESET ?
  
 
 Just like the regulator, reset_gpio_port has nothing to do with OMAP
 EHCI. So we would want to get rid of that too from the OMAP USB driver.

+1 to that.

That's a requirement from the LAN95xx hub controller, not OMAP EHCI ;-)

-- 
balbi


signature.asc
Description: Digital signature


Re: [RFC PATCH 1/5] drivers : introduce device_path api

2012-12-06 Thread Jassi Brar
On Wed, Dec 5, 2012 at 7:39 PM, Roger Quadros rog...@ti.com wrote:
 Hi Jassi,

 On 12/01/2012 09:49 AM, Jassi Brar wrote:
 On Tue, Nov 27, 2012 at 10:32 PM, Greg KH gre...@linuxfoundation.org wrote:
 On Tue, Nov 27, 2012 at 11:30:11AM -0500, Alan Stern wrote:

 We should have a more generic solution in a more central location, like
 the device core.

 For example, each struct platform_device could have a list of resources
 to be enabled when the device is bound to a driver and disabled when
 the device is unbound.  Such a list could include regulators, clocks,
 and whatever else people can invent.

 In this case, when it is created the ehci-omap.0 platform device could
 get an entry pointing to the LAN95xx's regulator.  Then the regulator
 would automatically be turned on when the platform device is bound to
 the ehci-omap driver.

 How does this sound?

 That sounds much better, and it might come in handy for other types of
 devices than platform devices, so feel free to put this on the core
 'struct device' instead if needed.

 Isn't enabling/disabling of clocks and regulators what
 dev.probe()/remove() of any driver already does?
 If we mean only board specific setup/teardown, still it would mean
 having the device core to do an extra 'probe' call when the current
 probe() is already meant to do whatever it takes to bring the device
 up. To my untrained eyes, it seem like messing with the
 probe()/remove()'s semantics.
  IMHO, if we really must implement something like that, may be we
 should employ some 'broadcast mechanism' so that anything anywhere in
 kernel knows which new device has been probed()/removed()
 successfully. I haven't thought exactly how because I am not sure even
 that would be the right way to approach PandaBoard's problem.

 Looking at Figure 15 – Panda USBB1 Interface Block Diagram of
 http://pandaboard.org/sites/default/files/board_reference/pandaboard-es-b/panda-es-b-manual.pdf
 gives me visions ...

  1) OMAP doesn't provide the USB root-hub, but only ULPIPHY. It is
 USB3320C chip that employs OMAP's ULPIPHY to provide the root-hub. So
 we should have a platform device for USB3320C, the probe() of which
 should simply

 Actually the EHCI controller within OMAP provides the root hub with 3
 ports but no PHY. One of them is connected to the USB3320 which is just
 a ULPI PHY.

a) Enable REFCLK (FREF_CLK3_OUT)
b) Reset itself by cycling RESETB (GPIO_62)
c) Create one ehci-omap platform device

 c) is not appropriate. ehci-omap must be created before usb3320.

  (or in appropriate order if the above isn't)
 That way insmod/rmmod'ing the USB3320C driver would power-up/down the
 whole subsystem.

 Yes, this is where we can think of a generic PHY driver to make sure thy
 PHY has necessary resources enabled. In the Panda case it would be the
 PHY clock and RESET gpio.

I wonder if ehci-omap should assume, besides regulator, a clock might
also be need to setup for the transceiver chip.
Maybe usbhs_bdata in board-omap4panda.c should have
.reset_gpio_port[0]  = GPIO_HUB_NRESET ?


 The LAN95xx chip still needs to be powered up and the PHY driver isn't
 the right place for that (though it could be done there as a hack). The
 closest we can get to emulating right USB behavior is to map the
 SET/CLEAR PORT_FEATURE of the root hub port to the regulator that powers
 the LAN95xx.

 This way, we still don't fall in the dynamically probed space, so we
 could still provide the regulator information to the ehci hub via
 platform data and handle the regulator in ehci_hub_control
 (Set/ClearPortFeature). I'm looking at this as a software workaround for
 all platforms not implementing the EHCI set port power feature
 correctly. The same could be repeated for other HCDs as well if required.

IMHO it's not a matter of platforms not implementing EHCI set port
power feature correctly, we should think about onboard devices
connected to onboard non-root hubs. Setups like LAN9514 on Panda (HSIC
too ?) that don't run on VBUS of USB, would like their local supply to
be treated as if it came from the parent hub's port  i.e, tie together
the USB's set port power control with their OOB regulated power supply
(U9 on PandaBoard).

cheers.
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH 1/5] drivers : introduce device_path api

2012-12-05 Thread Roger Quadros
Hi Jassi,

On 12/01/2012 09:49 AM, Jassi Brar wrote:
 On Tue, Nov 27, 2012 at 10:32 PM, Greg KH gre...@linuxfoundation.org wrote:
 On Tue, Nov 27, 2012 at 11:30:11AM -0500, Alan Stern wrote:

 We should have a more generic solution in a more central location, like
 the device core.

 For example, each struct platform_device could have a list of resources
 to be enabled when the device is bound to a driver and disabled when
 the device is unbound.  Such a list could include regulators, clocks,
 and whatever else people can invent.

 In this case, when it is created the ehci-omap.0 platform device could
 get an entry pointing to the LAN95xx's regulator.  Then the regulator
 would automatically be turned on when the platform device is bound to
 the ehci-omap driver.

 How does this sound?

 That sounds much better, and it might come in handy for other types of
 devices than platform devices, so feel free to put this on the core
 'struct device' instead if needed.

 Isn't enabling/disabling of clocks and regulators what
 dev.probe()/remove() of any driver already does?
 If we mean only board specific setup/teardown, still it would mean
 having the device core to do an extra 'probe' call when the current
 probe() is already meant to do whatever it takes to bring the device
 up. To my untrained eyes, it seem like messing with the
 probe()/remove()'s semantics.
  IMHO, if we really must implement something like that, may be we
 should employ some 'broadcast mechanism' so that anything anywhere in
 kernel knows which new device has been probed()/removed()
 successfully. I haven't thought exactly how because I am not sure even
 that would be the right way to approach PandaBoard's problem.
 
 Looking at Figure 15 – Panda USBB1 Interface Block Diagram of
 http://pandaboard.org/sites/default/files/board_reference/pandaboard-es-b/panda-es-b-manual.pdf
 gives me visions ...
 
  1) OMAP doesn't provide the USB root-hub, but only ULPIPHY. It is
 USB3320C chip that employs OMAP's ULPIPHY to provide the root-hub. So
 we should have a platform device for USB3320C, the probe() of which
 should simply

Actually the EHCI controller within OMAP provides the root hub with 3
ports but no PHY. One of them is connected to the USB3320 which is just
a ULPI PHY.

a) Enable REFCLK (FREF_CLK3_OUT)
b) Reset itself by cycling RESETB (GPIO_62)
c) Create one ehci-omap platform device

c) is not appropriate. ehci-omap must be created before usb3320.

  (or in appropriate order if the above isn't)
 That way insmod/rmmod'ing the USB3320C driver would power-up/down the
 whole subsystem.

Yes, this is where we can think of a generic PHY driver to make sure thy
PHY has necessary resources enabled. In the Panda case it would be the
PHY clock and RESET gpio.

The LAN95xx chip still needs to be powered up and the PHY driver isn't
the right place for that (though it could be done there as a hack). The
closest we can get to emulating right USB behavior is to map the
SET/CLEAR PORT_FEATURE of the root hub port to the regulator that powers
the LAN95xx.

This way, we still don't fall in the dynamically probed space, so we
could still provide the regulator information to the ehci hub via
platform data and handle the regulator in ehci_hub_control
(Set/ClearPortFeature). I'm looking at this as a software workaround for
all platforms not implementing the EHCI set port power feature
correctly. The same could be repeated for other HCDs as well if required.

cheers,
-roger
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH 1/5] drivers : introduce device_path api

2012-12-01 Thread Andy Green

On 12/01/2012 03:49 PM, the mail apparently from Jassi Brar included:

On Tue, Nov 27, 2012 at 10:32 PM, Greg KH gre...@linuxfoundation.org wrote:

On Tue, Nov 27, 2012 at 11:30:11AM -0500, Alan Stern wrote:


We should have a more generic solution in a more central location, like
the device core.

For example, each struct platform_device could have a list of resources
to be enabled when the device is bound to a driver and disabled when
the device is unbound.  Such a list could include regulators, clocks,
and whatever else people can invent.

In this case, when it is created the ehci-omap.0 platform device could
get an entry pointing to the LAN95xx's regulator.  Then the regulator
would automatically be turned on when the platform device is bound to
the ehci-omap driver.

How does this sound?


That sounds much better, and it might come in handy for other types of
devices than platform devices, so feel free to put this on the core
'struct device' instead if needed.


Isn't enabling/disabling of clocks and regulators what
dev.probe()/remove() of any driver already does?


The particular issue this tries to address is stuff that is not part of 
the generic driver function, but which is tied to the device instance by 
hardware connection choices on the physical platform.  External clocks, 
regulators and mux settings needed to make the device instance work on 
the board may all be out of scope for the generic driver, even when the 
generic driver has a SoC-specific part as in this case.


So no, enabling regulators, clock and mux it has no idea about because 
the dependency only exists as a board feature is not the job of probe / 
remove in drivers already.



If we mean only board specific setup/teardown, still it would mean
having the device core to do an extra 'probe' call when the current
probe() is already meant to do whatever it takes to bring the device
up. To my untrained eyes, it seem like messing with the
probe()/remove()'s semantics.


It's in the way of automating and simplifying code that would be 
repeated in each driver probe routine according to what you're 
suggesting.  In fact the pre-probe activity happens at the start of the 
actual probe code, and post remove at the end of the actual remove, 
there is no duplicated activity.  It's just a helper.



  IMHO, if we really must implement something like that, may be we
should employ some 'broadcast mechanism' so that anything anywhere in
kernel knows which new device has been probed()/removed()
successfully. I haven't thought exactly how because I am not sure even
that would be the right way to approach PandaBoard's problem.


I think this stuff is a bit more multifacted than you're giving it 
credit for, and indeed this thread has gone right into this aspect.  Say 
we hooked to a device creation notifier, we still must identify the 
correct device, in the case the device is on a dynamic bus.  Hence the 
discussion about device paths.  Just throwing a notifier at it itself 
doesn't scratch the problem.  The stuff I completed yesterday does solve 
this dynamic matching issue inexpensively.



Looking at Figure 15 – Panda USBB1 Interface Block Diagram of
http://pandaboard.org/sites/default/files/board_reference/pandaboard-es-b/panda-es-b-manual.pdf
gives me visions ...

  1) OMAP doesn't provide the USB root-hub, but only ULPIPHY. It is
USB3320C chip that employs OMAP's ULPIPHY to provide the root-hub. So
we should have a platform device for USB3320C, the probe() of which
should simply
a) Enable REFCLK (FREF_CLK3_OUT)
b) Reset itself by cycling RESETB (GPIO_62)
c) Create one ehci-omap platform device
  (or in appropriate order if the above isn't)
That way insmod/rmmod'ing the USB3320C driver would power-up/down the
whole subsystem.


First there's the issue that Panda has the same signal to reset the ULPI 
PHY and the SMSC device, and that we must operate that reset after 
powering the SMSC device.  The only nice way to do that is to arrange 
for the reset to happen once for both, at a time after the SMSC chip is 
powered, so we have no need to interrupt ULPI operation or touch the 
reset subsequently, until next powerup of the ULPI PHY + SMSC.


That is why tying foreign-to-the-generic-driver assets to a device 
instantiation can get us out of the hole, even when we want a hub port 
device that is impossible to have as a platform_device to control the 
power and clock for the SMSC device.


With Panda just trying to cleanly deal with ULPI reset in ULPI driver 
and SMSC reset in SMSC driver (how're you going to let that dynamically 
created smsc device know the gpio again?) are complicated by both 
getting reset by the same signal.


Second, the choices of Panda about providing a refclock and reset to the 
ULPI PHY are not SoC level choices but board level ones.



2) Just like the user has to manually power-on/off any externally
powered hub connected to a PC, maybe we should implement a user
controlled 'soft' switch (reacting to 

Re: [RFC PATCH 1/5] drivers : introduce device_path api

2012-11-30 Thread Jassi Brar
On Tue, Nov 27, 2012 at 10:32 PM, Greg KH gre...@linuxfoundation.org wrote:
 On Tue, Nov 27, 2012 at 11:30:11AM -0500, Alan Stern wrote:

 We should have a more generic solution in a more central location, like
 the device core.

 For example, each struct platform_device could have a list of resources
 to be enabled when the device is bound to a driver and disabled when
 the device is unbound.  Such a list could include regulators, clocks,
 and whatever else people can invent.

 In this case, when it is created the ehci-omap.0 platform device could
 get an entry pointing to the LAN95xx's regulator.  Then the regulator
 would automatically be turned on when the platform device is bound to
 the ehci-omap driver.

 How does this sound?

 That sounds much better, and it might come in handy for other types of
 devices than platform devices, so feel free to put this on the core
 'struct device' instead if needed.

Isn't enabling/disabling of clocks and regulators what
dev.probe()/remove() of any driver already does?
If we mean only board specific setup/teardown, still it would mean
having the device core to do an extra 'probe' call when the current
probe() is already meant to do whatever it takes to bring the device
up. To my untrained eyes, it seem like messing with the
probe()/remove()'s semantics.
 IMHO, if we really must implement something like that, may be we
should employ some 'broadcast mechanism' so that anything anywhere in
kernel knows which new device has been probed()/removed()
successfully. I haven't thought exactly how because I am not sure even
that would be the right way to approach PandaBoard's problem.

Looking at Figure 15 – Panda USBB1 Interface Block Diagram of
http://pandaboard.org/sites/default/files/board_reference/pandaboard-es-b/panda-es-b-manual.pdf
gives me visions ...

 1) OMAP doesn't provide the USB root-hub, but only ULPIPHY. It is
USB3320C chip that employs OMAP's ULPIPHY to provide the root-hub. So
we should have a platform device for USB3320C, the probe() of which
should simply
   a) Enable REFCLK (FREF_CLK3_OUT)
   b) Reset itself by cycling RESETB (GPIO_62)
   c) Create one ehci-omap platform device
 (or in appropriate order if the above isn't)
That way insmod/rmmod'ing the USB3320C driver would power-up/down the
whole subsystem.

2) Just like the user has to manually power-on/off any externally
powered hub connected to a PC, maybe we should implement a user
controlled 'soft' switch (reacting to UDEV events from ehci-omap?) to
emulate LAN9514 power-on/off. I do realize it would be cool to have it
automatically toggle in kernel when we (de)power the hub but isn't
that outside of scope of Linux USB implementation?

The above solution isn't most optimal for Panda but it seems a design
more consistent with what we already have. Or so do I think.

Cheers.
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH 1/5] drivers : introduce device_path api

2012-11-29 Thread Alan Stern
On Thu, 29 Nov 2012, Ming Lei wrote:

 If we want to set up the association between usb port and power domain,
 usb knowledge is required, at least bus info and port topology are needed.
 
 One difficulty is the fact that the device(such as usb port) is independent
 with the 'power domain', for example, the device isn't created(ports of the
 root hub is created after ehci-omap probe, and port device of non-root
 hub may depend on powering on the power domain) when defining the regulator
 things, so could we figure out one general way in theory to describe the
 associated device with the 'power domain'? Andy has tried the wildcard dev
 path, and port topology string is introduced in my suggestion, looks both
 are not general.

The device path approach probably could be made to work, but it does
have the problem that it depends on device names which may change in
the future.  However, I can't think of any other general-purpose
approach.  And most especially, I can't think of any approach that
doesn't require extra overhead _every_ time a new device is registered.

The port topology string is, of course, specific to USB.

 I admit the suggestion is partial because we still have not a general abstract
 on 'power domain' in this problem, and once it is ready, the solution might be
 in a shape at least for USB. In PC world, ACPI might do sort of something of
 the 'power domain'

I'm not worried about the power domain part.  For now, a regulator is 
all we need.  It should be easy enough to expand that later on.

 Maybe we need to create a new thread on this discussion and make more
 guys involved(PM/USB/driver core/OMAP/). I will study the problem further,
 and hope I can post something for starting the discussion later.
 
  It would be better to have a more general approach.  So far nobody has
 
 Yes, I agree. IMO, my suggestion is still in the direction to being general,
 because a general 'power domain' concept is introduced in it, for example
 the 'struct power_domain' is associated with 'struct port_power_domain'.

It's general, but in the wrong direction.  The hard part isn't the 
power domain aspect; it is setting up the match between the power 
domain and the dynamically created device.

 I understand the same 'power domain' concept should be applied to other
 device or bus too, and the power associated with this device can be switched 
 off
 sometimes too for saving power consumption. But still looks specific
 device/subsystem knowledge is required to set up the association.
 
 Alan, so could the above be your concern on a more general approach?
 Or you hope a more general way(such as, do it in driver core or dev PM core)
 to associate the 'power domain' with one device(for example, usb port in
 the LAN95xx problem) too? Or other things?

Right now we don't have _any_ general (i.e., not bus-specific) way to 
identify individual devices except for the device name strings.

I don't know -- maybe this shouldn't have a general solution at all.  
Maybe we just need a platform-specific way to associate a regulator or
clock with a USB port, something that the port driver would know about
explicitly.

Alan Stern

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH 1/5] drivers : introduce device_path api

2012-11-28 Thread Roger Quadros
On 11/27/2012 09:22 PM, Andy Green wrote:
 On 11/28/2012 02:09 AM, the mail apparently from Alan Stern included:
 On Wed, 28 Nov 2012, Andy Green wrote:

 Greg's advice was simply not to rely on pathnames in sysfs because they
 aren't fixed in stone.  That leaves plenty of other ways to approach
 this problem.

 It's sage advice, but there is zero code provided in my patches that
 relies on pathnames in sysfs.

 In your 1/5 patch, _device_path_generate() concatenates device name
 strings, starting from a device root and separating elements with '/'
 characters.  Isn't that the same as a sysfs pathname?
 
 It's nothing to do with sysfs... yes some unrelated bits of sysfs also
 walk the device path.  If we want to talk about how fragile the device
 path is as an id scheme over time we need to talk about likelihood of
 individual device names changing, not sysfs.  Anyway --
 
 Basically, what you want is for something related to device A (the
 regulator or the GPIO) to happen whenever device B (the ehci-omap.0
 platform device) is bound to a driver.  The most straightforward way to
 arrange this is for A's driver to have a callback that is invoked
 whenever B is bound or unbound.  The most straightforward way to
 arrange _that_ is to allow each platform_device to have a list of
 callbacks.

 Sorry I didn't really understand this proposal yet.  You want A, the
 regulator, driver to grow a callback function that gets called when the
 targeted platform_device (B, ehci-omap.0) probe happens.  Could you
 expand what the callback prototype or new members in the struct might
 look like?  It's your tuple thing or we pass it an opaque pointer that
 is the struct regulator * or suchlike?

 Well, it won't be exactly the same as the tuple thing because no
 strings will be involved, but it would be similar.  The callback would
 receive an opaque pointer (presumably to the regulator) and a device
 pointer (the B device).
 
 OK.  So I try to sketch it out iteractively to try to get in sync:
 
 device.h:
 
 enum asset_event {
 AE_PROBED,
 AE_REMOVED
 };
 
 struct device_asset {
 char *name; /* name of regulator, clock, etc */
 void *asset; /* regulator, clock, etc */
 int (*handler)(struct device *dev_owner, enum asset_event
 asset_event, struct device_asset *asset);
 };
 
 struct device {
 ...
 struct device_asset *assets;
 ...
 };
 
 
 drivers/base/dd.c | really_probe():
 
 ...
 struct device_asset *asset;
 ...
 asset = dev-assets;
 while (asset  asset-name) {
 if (asset-handler(dev, AE_PROBED, asset)) {
 /* clean up and bail */
 }
 asset++;
 }
 
 /* do probe */
 ...
 
 
 drivers/base/dd.c | __device_release_driver:  (is this really the best
 place to oppose probe()?)
 
 ...
 struct device_asset *asset;
 ...
 
 /* call device -remove() */
 ...
 asset = dev-assets;
 while (asset  asset-name) {
 asset-handler(dev, AE_REMOVED, asset);
 asset++;
 }
 ...
 
 
 board file:
 
 static struct regulator myreg = {
 .name = mydevice-regulator,
 };
 
 static struct device_asset mydevice_assets[] = {
 {
 .name = mydevice-regulator,
 .handler = regulator_default_asset_handler,
 },
 { }
 };
 
 static struct platform_device mydevice = {
 ...
 .dev = {
 .assets = mydevice_assets,
 },
 ...
 };
 

From Pandaboard's point of view, is mydevice supposed to be referring to
ehci-omap, LAN95xx or something else?

Strictly speaking, the regulator doesn't belongs neither to ehci-omap
nor LAN95xx. It belongs to a power domain on the board. And user should
have control to switch it OFF when required without hampering operation
of ehci-omap, so that the other USB ports are still usable.

--
regards,
-roger

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH 1/5] drivers : introduce device_path api

2012-11-28 Thread Andy Green

On 11/28/2012 07:13 PM, the mail apparently from Roger Quadros included:

On 11/27/2012 09:22 PM, Andy Green wrote:

On 11/28/2012 02:09 AM, the mail apparently from Alan Stern included:

On Wed, 28 Nov 2012, Andy Green wrote:


Greg's advice was simply not to rely on pathnames in sysfs because they
aren't fixed in stone.  That leaves plenty of other ways to approach
this problem.


It's sage advice, but there is zero code provided in my patches that
relies on pathnames in sysfs.


In your 1/5 patch, _device_path_generate() concatenates device name
strings, starting from a device root and separating elements with '/'
characters.  Isn't that the same as a sysfs pathname?


It's nothing to do with sysfs... yes some unrelated bits of sysfs also
walk the device path.  If we want to talk about how fragile the device
path is as an id scheme over time we need to talk about likelihood of
individual device names changing, not sysfs.  Anyway --


Basically, what you want is for something related to device A (the
regulator or the GPIO) to happen whenever device B (the ehci-omap.0
platform device) is bound to a driver.  The most straightforward way to
arrange this is for A's driver to have a callback that is invoked
whenever B is bound or unbound.  The most straightforward way to
arrange _that_ is to allow each platform_device to have a list of
callbacks.


Sorry I didn't really understand this proposal yet.  You want A, the
regulator, driver to grow a callback function that gets called when the
targeted platform_device (B, ehci-omap.0) probe happens.  Could you
expand what the callback prototype or new members in the struct might
look like?  It's your tuple thing or we pass it an opaque pointer that
is the struct regulator * or suchlike?


Well, it won't be exactly the same as the tuple thing because no
strings will be involved, but it would be similar.  The callback would
receive an opaque pointer (presumably to the regulator) and a device
pointer (the B device).


OK.  So I try to sketch it out iteractively to try to get in sync:

device.h:

 enum asset_event {
 AE_PROBED,
 AE_REMOVED
 };

 struct device_asset {
 char *name; /* name of regulator, clock, etc */
 void *asset; /* regulator, clock, etc */
 int (*handler)(struct device *dev_owner, enum asset_event
asset_event, struct device_asset *asset);
 };

 struct device {
 ...
 struct device_asset *assets;
 ...
 };


drivers/base/dd.c | really_probe():

...
 struct device_asset *asset;
...
 asset = dev-assets;
 while (asset  asset-name) {
 if (asset-handler(dev, AE_PROBED, asset)) {
 /* clean up and bail */
 }
 asset++;
 }

 /* do probe */
...


drivers/base/dd.c | __device_release_driver:  (is this really the best
place to oppose probe()?)

...
 struct device_asset *asset;
...

 /* call device -remove() */
...
 asset = dev-assets;
 while (asset  asset-name) {
 asset-handler(dev, AE_REMOVED, asset);
 asset++;
 }
...


board file:

 static struct regulator myreg = {
 .name = mydevice-regulator,
 };

 static struct device_asset mydevice_assets[] = {
 {
 .name = mydevice-regulator,
 .handler = regulator_default_asset_handler,
 },
 { }
 };

 static struct platform_device mydevice = {
 ...
 .dev = {
 .assets = mydevice_assets,
 },
 ...
 };



 From Pandaboard's point of view, is mydevice supposed to be referring to
ehci-omap, LAN95xx or something else?

Strictly speaking, the regulator doesn't belongs neither to ehci-omap
nor LAN95xx. It belongs to a power domain on the board. And user should
have control to switch it OFF when required without hampering operation
of ehci-omap, so that the other USB ports are still usable.


I'd prefer to deal with that after a try#1 with regulator, I didn't see 
any code about power domain in there right now.  There's a lot to get 
consensus on already with this series.


Notice that these assets are generic, I will provide clk and regulator 
handlers with try#1, and working examples for Panda case with both, but 
presumably power domain can fold into it as well.


Since I am on usb-next and there's nothing to be seen about power 
domains, what is the situation with that support?


-Andy

--
Andy Green | TI Landing Team Leader
Linaro.org │ Open source software for ARM SoCs | Follow Linaro
http://facebook.com/pages/Linaro/155974581091106  - 
http://twitter.com/#!/linaroorg - http://linaro.org/linaro-blog

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH 1/5] drivers : introduce device_path api

2012-11-28 Thread Roger Quadros
On 11/28/2012 01:47 PM, Andy Green wrote:
 On 11/28/2012 07:13 PM, the mail apparently from Roger Quadros included:
 On 11/27/2012 09:22 PM, Andy Green wrote:
 On 11/28/2012 02:09 AM, the mail apparently from Alan Stern included:
 On Wed, 28 Nov 2012, Andy Green wrote:

 Greg's advice was simply not to rely on pathnames in sysfs because
 they
 aren't fixed in stone.  That leaves plenty of other ways to approach
 this problem.

 It's sage advice, but there is zero code provided in my patches that
 relies on pathnames in sysfs.

 In your 1/5 patch, _device_path_generate() concatenates device name
 strings, starting from a device root and separating elements with '/'
 characters.  Isn't that the same as a sysfs pathname?

 It's nothing to do with sysfs... yes some unrelated bits of sysfs also
 walk the device path.  If we want to talk about how fragile the device
 path is as an id scheme over time we need to talk about likelihood of
 individual device names changing, not sysfs.  Anyway --

 Basically, what you want is for something related to device A (the
 regulator or the GPIO) to happen whenever device B (the ehci-omap.0
 platform device) is bound to a driver.  The most straightforward
 way to
 arrange this is for A's driver to have a callback that is invoked
 whenever B is bound or unbound.  The most straightforward way to
 arrange _that_ is to allow each platform_device to have a list of
 callbacks.

 Sorry I didn't really understand this proposal yet.  You want A, the
 regulator, driver to grow a callback function that gets called when
 the
 targeted platform_device (B, ehci-omap.0) probe happens.  Could you
 expand what the callback prototype or new members in the struct might
 look like?  It's your tuple thing or we pass it an opaque pointer that
 is the struct regulator * or suchlike?

 Well, it won't be exactly the same as the tuple thing because no
 strings will be involved, but it would be similar.  The callback would
 receive an opaque pointer (presumably to the regulator) and a device
 pointer (the B device).

 OK.  So I try to sketch it out iteractively to try to get in sync:

 device.h:

  enum asset_event {
  AE_PROBED,
  AE_REMOVED
  };

  struct device_asset {
  char *name; /* name of regulator, clock, etc */
  void *asset; /* regulator, clock, etc */
  int (*handler)(struct device *dev_owner, enum asset_event
 asset_event, struct device_asset *asset);
  };

  struct device {
  ...
  struct device_asset *assets;
  ...
  };


 drivers/base/dd.c | really_probe():

 ...
  struct device_asset *asset;
 ...
  asset = dev-assets;
  while (asset  asset-name) {
  if (asset-handler(dev, AE_PROBED, asset)) {
  /* clean up and bail */
  }
  asset++;
  }

  /* do probe */
 ...


 drivers/base/dd.c | __device_release_driver:  (is this really the best
 place to oppose probe()?)

 ...
  struct device_asset *asset;
 ...

  /* call device -remove() */
 ...
  asset = dev-assets;
  while (asset  asset-name) {
  asset-handler(dev, AE_REMOVED, asset);
  asset++;
  }
 ...


 board file:

  static struct regulator myreg = {
  .name = mydevice-regulator,
  };

  static struct device_asset mydevice_assets[] = {
  {
  .name = mydevice-regulator,
  .handler = regulator_default_asset_handler,
  },
  { }
  };

  static struct platform_device mydevice = {
  ...
  .dev = {
  .assets = mydevice_assets,
  },
  ...
  };


  From Pandaboard's point of view, is mydevice supposed to be referring to
 ehci-omap, LAN95xx or something else?

 Strictly speaking, the regulator doesn't belongs neither to ehci-omap
 nor LAN95xx. It belongs to a power domain on the board. And user should
 have control to switch it OFF when required without hampering operation
 of ehci-omap, so that the other USB ports are still usable.
 
 I'd prefer to deal with that after a try#1 with regulator, I didn't see
 any code about power domain in there right now.  There's a lot to get
 consensus on already with this series.

Sure. I meant power domain in the general sense and not referring to any
code or framework in the kernel. It could as well be implemented using
the existing regulator framework.

 
 Notice that these assets are generic, I will provide clk and regulator
 handlers with try#1, and working examples for Panda case with both, but
 presumably power domain can fold into it as well.
 
 Since I am on usb-next and there's nothing to be seen about power
 domains, what is the situation with that support?
 

Looking around there seems to be some power domain framework tied to
runtime PM in drivers/base/power/domain.c

The idea is that the power domain is powered on/off by the runtime PM
core when the device is runtime resumed/suspended. I think this is meant
for SoC 

Re: [RFC PATCH 1/5] drivers : introduce device_path api

2012-11-28 Thread Alan Stern
On Wed, 28 Nov 2012, Roger Quadros wrote:

  board file:
  
  static struct regulator myreg = {
  .name = mydevice-regulator,
  };
  
  static struct device_asset mydevice_assets[] = {
  {
  .name = mydevice-regulator,
  .handler = regulator_default_asset_handler,
  },
  { }
  };
  
  static struct platform_device mydevice = {
  ...
  .dev = {
  .assets = mydevice_assets,
  },
  ...
  };
  
 
 From Pandaboard's point of view, is mydevice supposed to be referring to
 ehci-omap, LAN95xx or something else?

ehci-omap.0.

 Strictly speaking, the regulator doesn't belongs neither to ehci-omap
 nor LAN95xx. It belongs to a power domain on the board. And user should
 have control to switch it OFF when required without hampering operation
 of ehci-omap, so that the other USB ports are still usable.

That is the one disadvantage of the approach we are discussing.

But what API would you use to give the user this control?  Neither the
GPIO nor the regulator has a struct device, so you can't use sysfs
directly.  And even if you could, it would probably be a bad idea
because the user might turn off power to the LAN95xx while the chip was
being used.

The natural answer is to associate the regulator with the USB port that 
the LAN95xx is connected to, so that the new port-power mechanism could 
provide the control you want.  Then how should that association be set 
up?

Lei Ming provided a partial answer, but his suggestion is tied to USB.  
It would be better to have a more general approach.  So far nobody has 
come up with a suggestion that everybody approves of.

Alan Stern

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH 1/5] drivers : introduce device_path api

2012-11-28 Thread Ming Lei
On Thu, Nov 29, 2012 at 12:43 AM, Alan Stern st...@rowland.harvard.edu wrote:
 On Wed, 28 Nov 2012, Roger Quadros wrote:

  board file:
 
  static struct regulator myreg = {
  .name = mydevice-regulator,
  };
 
  static struct device_asset mydevice_assets[] = {
  {
  .name = mydevice-regulator,
  .handler = regulator_default_asset_handler,
  },
  { }
  };
 
  static struct platform_device mydevice = {
  ...
  .dev = {
  .assets = mydevice_assets,
  },
  ...
  };
 

 From Pandaboard's point of view, is mydevice supposed to be referring to
 ehci-omap, LAN95xx or something else?

 ehci-omap.0.

 Strictly speaking, the regulator doesn't belongs neither to ehci-omap
 nor LAN95xx. It belongs to a power domain on the board. And user should
 have control to switch it OFF when required without hampering operation
 of ehci-omap, so that the other USB ports are still usable.

 That is the one disadvantage of the approach we are discussing.

 But what API would you use to give the user this control?  Neither the
 GPIO nor the regulator has a struct device, so you can't use sysfs
 directly.  And even if you could, it would probably be a bad idea
 because the user might turn off power to the LAN95xx while the chip was
 being used.

After Tianyu introduced the power power on/off mechanism, sometimes
one port power need to be switched off/on. Embedded system is more
power sensitive than PC, sounds we have no reason to reject applying
the mechanism on embedded world(non ACPI). Looks better to associate
the power domain thing(regulator, clock, ...) with one usb port device in
this USB problem.


 The natural answer is to associate the regulator with the USB port that
 the LAN95xx is connected to, so that the new port-power mechanism could
 provide the control you want.  Then how should that association be set
 up?

As I suggested in below link, the association can be set up easily with one
structure of 'struct port_power_domain'.

 http://www.spinics.net/lists/linux-omap/msg83158.html


 Lei Ming provided a partial answer, but his suggestion is tied to USB.

If we want to set up the association between usb port and power domain,
usb knowledge is required, at least bus info and port topology are needed.

One difficulty is the fact that the device(such as usb port) is independent
with the 'power domain', for example, the device isn't created(ports of the
root hub is created after ehci-omap probe, and port device of non-root
hub may depend on powering on the power domain) when defining the regulator
things, so could we figure out one general way in theory to describe the
associated device with the 'power domain'? Andy has tried the wildcard dev
path, and port topology string is introduced in my suggestion, looks both
are not general.

I admit the suggestion is partial because we still have not a general abstract
on 'power domain' in this problem, and once it is ready, the solution might be
in a shape at least for USB. In PC world, ACPI might do sort of something of
the 'power domain'

Maybe we need to create a new thread on this discussion and make more
guys involved(PM/USB/driver core/OMAP/). I will study the problem further,
and hope I can post something for starting the discussion later.

 It would be better to have a more general approach.  So far nobody has

Yes, I agree. IMO, my suggestion is still in the direction to being general,
because a general 'power domain' concept is introduced in it, for example
the 'struct power_domain' is associated with 'struct port_power_domain'.

I understand the same 'power domain' concept should be applied to other
device or bus too, and the power associated with this device can be switched off
sometimes too for saving power consumption. But still looks specific
device/subsystem knowledge is required to set up the association.

Alan, so could the above be your concern on a more general approach?
Or you hope a more general way(such as, do it in driver core or dev PM core)
to associate the 'power domain' with one device(for example, usb port in
the LAN95xx problem) too? Or other things?


Thanks,
-- 
Ming Lei
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH 1/5] drivers : introduce device_path api

2012-11-27 Thread Alan Stern
On Tue, 27 Nov 2012, Ming Lei wrote:

 IMO, all matches mean the devices are inside the ehci-omap bus, so
 the direct/simple way is to enable/disable the regulators in the probe() and
 remove() of ehci-omap controller driver.

When this discussion started, Felipe argued against such an approach.  
He pointed out that the same chip could be used in other platforms, and
then the code added to ehci-omap.c would have to be duplicated in
several other places.

We should have a more generic solution in a more central location, like 
the device core.

For example, each struct platform_device could have a list of resources
to be enabled when the device is bound to a driver and disabled when
the device is unbound.  Such a list could include regulators, clocks,
and whatever else people can invent.

In this case, when it is created the ehci-omap.0 platform device could
get an entry pointing to the LAN95xx's regulator.  Then the regulator 
would automatically be turned on when the platform device is bound to 
the ehci-omap driver.

How does this sound?

Alan Stern

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH 1/5] drivers : introduce device_path api

2012-11-27 Thread Alan Stern
On Tue, 27 Nov 2012, Andy Green wrote:

  I don't know if such an approach would be sufficiently general for
  future requirements, but it would solve the problem at hand.
 
 We can get a notification about device creation and do things there. 
 But the cost of that is like the tuple suggestion, they'll only be able 
 to do a fixed thing like operate on regulators.

I'm not quite sure what you mean by that.  _Any_ function is capable of 
doing only a fixed thing.

 Actually the targeted device may have arbitrary associated assets like 
 generic GPIO to turn on a flash for built-in webcam controlled 
 out-of-band.  These also can be reached by known names.  And the driver 
 may wish to do things with them that are more complex than enable / 
 disable or follow logical lifecycle of the hub or whatever.

Let's worry about that when it arises.

 However when the guidance from Greg is that we can do nothing except 
 complain at hardware designers for some reason I failed to quite 
 identify, I fear we are moving deckchairs on the titanic.

Greg's advice was simply not to rely on pathnames in sysfs because they 
aren't fixed in stone.  That leaves plenty of other ways to approach 
this problem.

Basically, what you want is for something related to device A (the
regulator or the GPIO) to happen whenever device B (the ehci-omap.0
platform device) is bound to a driver.  The most straightforward way to
arrange this is for A's driver to have a callback that is invoked
whenever B is bound or unbound.  The most straightforward way to 
arrange _that_ is to allow each platform_device to have a list of 
callbacks.

Alan Stern

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH 1/5] drivers : introduce device_path api

2012-11-27 Thread Greg KH
On Tue, Nov 27, 2012 at 11:30:11AM -0500, Alan Stern wrote:
 On Tue, 27 Nov 2012, Ming Lei wrote:
 
  IMO, all matches mean the devices are inside the ehci-omap bus, so
  the direct/simple way is to enable/disable the regulators in the probe() and
  remove() of ehci-omap controller driver.
 
 When this discussion started, Felipe argued against such an approach.  
 He pointed out that the same chip could be used in other platforms, and
 then the code added to ehci-omap.c would have to be duplicated in
 several other places.
 
 We should have a more generic solution in a more central location, like 
 the device core.
 
 For example, each struct platform_device could have a list of resources
 to be enabled when the device is bound to a driver and disabled when
 the device is unbound.  Such a list could include regulators, clocks,
 and whatever else people can invent.
 
 In this case, when it is created the ehci-omap.0 platform device could
 get an entry pointing to the LAN95xx's regulator.  Then the regulator 
 would automatically be turned on when the platform device is bound to 
 the ehci-omap driver.
 
 How does this sound?

That sounds much better, and it might come in handy for other types of
devices than platform devices, so feel free to put this on the core
'struct device' instead if needed.

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH 1/5] drivers : introduce device_path api

2012-11-27 Thread Andy Green

On 11/28/2012 12:37 AM, the mail apparently from Alan Stern included:

On Tue, 27 Nov 2012, Andy Green wrote:


I don't know if such an approach would be sufficiently general for
future requirements, but it would solve the problem at hand.


We can get a notification about device creation and do things there.
But the cost of that is like the tuple suggestion, they'll only be able
to do a fixed thing like operate on regulators.


I'm not quite sure what you mean by that.  _Any_ function is capable of
doing only a fixed thing.


Actually the targeted device may have arbitrary associated assets like
generic GPIO to turn on a flash for built-in webcam controlled
out-of-band.  These also can be reached by known names.  And the driver
may wish to do things with them that are more complex than enable /
disable or follow logical lifecycle of the hub or whatever.


Let's worry about that when it arises.


However when the guidance from Greg is that we can do nothing except
complain at hardware designers for some reason I failed to quite
identify, I fear we are moving deckchairs on the titanic.


Greg's advice was simply not to rely on pathnames in sysfs because they
aren't fixed in stone.  That leaves plenty of other ways to approach
this problem.


It's sage advice, but there is zero code provided in my patches that 
relies on pathnames in sysfs.



Basically, what you want is for something related to device A (the
regulator or the GPIO) to happen whenever device B (the ehci-omap.0
platform device) is bound to a driver.  The most straightforward way to
arrange this is for A's driver to have a callback that is invoked
whenever B is bound or unbound.  The most straightforward way to
arrange _that_ is to allow each platform_device to have a list of
callbacks.


Sorry I didn't really understand this proposal yet.  You want A, the 
regulator, driver to grow a callback function that gets called when the 
targeted platform_device (B, ehci-omap.0) probe happens.  Could you 
expand what the callback prototype or new members in the struct might 
look like?  It's your tuple thing or we pass it an opaque pointer that 
is the struct regulator * or suchlike?


Throwing out the path stuff and limiting this to platform_device means 
you cannot bind to dynamically created objects like hub or anything 
downstream of a hub.  So Felipe's identification of the hub as the 
happening place to do this is out of luck.


-Andy

--
Andy Green | TI Landing Team Leader
Linaro.org │ Open source software for ARM SoCs | Follow Linaro
http://facebook.com/pages/Linaro/155974581091106  - 
http://twitter.com/#!/linaroorg - http://linaro.org/linaro-blog

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH 1/5] drivers : introduce device_path api

2012-11-27 Thread Andy Green

On 11/28/2012 01:22 AM, the mail apparently from Ming Lei included:

On Wed, Nov 28, 2012 at 12:30 AM, Alan Stern st...@rowland.harvard.edu wrote:

On Tue, 27 Nov 2012, Ming Lei wrote:


IMO, all matches mean the devices are inside the ehci-omap bus, so
the direct/simple way is to enable/disable the regulators in the probe() and
remove() of ehci-omap controller driver.


When this discussion started, Felipe argued against such an approach.
He pointed out that the same chip could be used in other platforms, and
then the code added to ehci-omap.c would have to be duplicated in
several other places.


 From Andy's implementation, the 'general' code is to enable/disable
the regulators(patch 3/5), I am wondering if it is general enough, so the
'duplicated' code are just several lines of regulator_enable/regulator_disable,
which should be implemented in many platform drivers.


Fair enough; my main interest was in the device path side when writing 
the patches.  I stuck enough in one place to confirm the series works on 
Panda case for power control.  So long as it doesn't get obsessed with 
just regulators some common implementation that seems to be discussed 
will be better.


(BTW let me take this opportunity to thank you for your great 
urbs-in-flight limiting patch on smsc95xx a while back)



Also from my intuition, power domain should be involved in the problem,
because these hard-wired and self-powered USB devices should have
its own power domains, and the ehci-omap driver may enable/disable
these power domains before adding the bus.


I don't know enough to have an opinion, but the arrangement on Panda is 
literally a linear regulator is being controlled by a gpio, which fits 
into struct regulator model.  What else would a power domain for that 
buy us?



We should have a more generic solution in a more central location, like
the device core.

For example, each struct platform_device could have a list of resources
to be enabled when the device is bound to a driver and disabled when
the device is unbound.  Such a list could include regulators, clocks,
and whatever else people can invent.

In this case, when it is created the ehci-omap.0 platform device could
get an entry pointing to the LAN95xx's regulator.  Then the regulator
would automatically be turned on when the platform device is bound to
the ehci-omap driver.


The LAN95xx's regulator is still platform dependent(even for same LAN95xx
USB devices on different platforms(omap, tegra, ..)) , so I am wondering
why we don't enable it directly in probe() of ehci-omap.0 platform device?


I didn't get this point, ehci-omap doesn't help if it's non-omap platform.

-Andy

--
Andy Green | TI Landing Team Leader
Linaro.org │ Open source software for ARM SoCs | Follow Linaro
http://facebook.com/pages/Linaro/155974581091106  - 
http://twitter.com/#!/linaroorg - http://linaro.org/linaro-blog

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH 1/5] drivers : introduce device_path api

2012-11-27 Thread Andy Green

On 11/28/2012 02:09 AM, the mail apparently from Alan Stern included:

On Wed, 28 Nov 2012, Andy Green wrote:


Greg's advice was simply not to rely on pathnames in sysfs because they
aren't fixed in stone.  That leaves plenty of other ways to approach
this problem.


It's sage advice, but there is zero code provided in my patches that
relies on pathnames in sysfs.


In your 1/5 patch, _device_path_generate() concatenates device name
strings, starting from a device root and separating elements with '/'
characters.  Isn't that the same as a sysfs pathname?


It's nothing to do with sysfs... yes some unrelated bits of sysfs also 
walk the device path.  If we want to talk about how fragile the device 
path is as an id scheme over time we need to talk about likelihood of 
individual device names changing, not sysfs.  Anyway --



Basically, what you want is for something related to device A (the
regulator or the GPIO) to happen whenever device B (the ehci-omap.0
platform device) is bound to a driver.  The most straightforward way to
arrange this is for A's driver to have a callback that is invoked
whenever B is bound or unbound.  The most straightforward way to
arrange _that_ is to allow each platform_device to have a list of
callbacks.


Sorry I didn't really understand this proposal yet.  You want A, the
regulator, driver to grow a callback function that gets called when the
targeted platform_device (B, ehci-omap.0) probe happens.  Could you
expand what the callback prototype or new members in the struct might
look like?  It's your tuple thing or we pass it an opaque pointer that
is the struct regulator * or suchlike?


Well, it won't be exactly the same as the tuple thing because no
strings will be involved, but it would be similar.  The callback would
receive an opaque pointer (presumably to the regulator) and a device
pointer (the B device).


OK.  So I try to sketch it out iteractively to try to get in sync:

device.h:

enum asset_event {
AE_PROBED,
AE_REMOVED
};

struct device_asset {
char *name; /* name of regulator, clock, etc */
void *asset; /* regulator, clock, etc */
		int (*handler)(struct device *dev_owner, enum asset_event asset_event, 
struct device_asset *asset);

};

struct device {
...
struct device_asset *assets;
...
};


drivers/base/dd.c | really_probe():

...
struct device_asset *asset;
...
asset = dev-assets;
while (asset  asset-name) {
if (asset-handler(dev, AE_PROBED, asset)) {
/* clean up and bail */
}
asset++;
}

/* do probe */
...


drivers/base/dd.c | __device_release_driver:  (is this really the best 
place to oppose probe()?)


...
struct device_asset *asset;
...

/* call device -remove() */
...
asset = dev-assets;
while (asset  asset-name) {
asset-handler(dev, AE_REMOVED, asset);
asset++;
}
...


board file:

static struct regulator myreg = {
.name = mydevice-regulator,
};

static struct device_asset mydevice_assets[] = {
{
.name = mydevice-regulator,
.handler = regulator_default_asset_handler,
},
{ }
};

static struct platform_device mydevice = {
...
.dev = {
.assets = mydevice_assets,
},
...
};


regulator code:

int regulator_default_asset_handler(struct device *dev_owner, enum 
asset_event asset_event, struct device_asset *asset)

{
struct regulator **reg = asset-asset;
int n;

switch (asset_event) {
case AE_PROBED:
*reg = regulator_get(dev_owner, asset-name);
if (IS_ERR(*reg))
return *reg;
n = regulator_enable(*reg);
if (n  0)
regulator_put(*reg);
return n;

case AE_REMOVED:
regulator_put(*reg);
break;
}

return 0;
}
EXPORT_SYMBOL_GPL(regulator_default_asset_handler);


The subsystems that can expect to get used (clock...) might each want to 
define a default handler like the one for regulator.  That'll be an end 
to the code duplication issue.  The user can still do his own handler if 
he wants.


I put a name field in so we can use regulator_get() nicely, we don't 
need access to the object pointer or that it exists at boardfile-time 
that way either.  But I can see it's arguable.



Throwing out the path stuff and limiting this to platform_device means
you cannot bind to dynamically created objects like hub or anything
downstream of a hub.  So Felipe's identification of the hub as the
happening place to do 

Re: [RFC PATCH 1/5] drivers : introduce device_path api

2012-11-27 Thread Alan Stern
On Wed, 28 Nov 2012, Andy Green wrote:

 OK.  So I try to sketch it out iteractively to try to get in sync:
 
 device.h:
 
   enum asset_event {
   AE_PROBED,
   AE_REMOVED
   };
 
   struct device_asset {
   char *name; /* name of regulator, clock, etc */
   void *asset; /* regulator, clock, etc */
   int (*handler)(struct device *dev_owner, enum asset_event 
 asset_event, 
 struct device_asset *asset);
   };

Another possibility is to have two handlers: one for pre-probe and the
other for post-remove.  Then of course asset_event wouldn't be needed.  
Linus tends to prefer this strategy -- separate functions for separate
events.  That's why struct dev_pm_ops has so many method pointers.

   struct device {
   ...
   struct device_asset *assets;

Or a list instead of a NULL-terminated array.  It depends on whether
people will want to add or remove assets dynamically.  For now an array
would be fine.

   ...
   };
 
 
 drivers/base/dd.c | really_probe():
 
 ...
   struct device_asset *asset;
 ...
   asset = dev-assets;
   while (asset  asset-name) {

Maybe it would be better to test asset-handler.  Some assets might not 
need names.

   if (asset-handler(dev, AE_PROBED, asset)) {
   /* clean up and bail */
   }
   asset++;
   }
 
   /* do probe */
 ...
 
 
 drivers/base/dd.c | __device_release_driver:  (is this really the best 
 place to oppose probe()?)

The right place is just after the remove method is called, so yes.

 ...
   struct device_asset *asset;
 ...
 
   /* call device -remove() */
 ...
   asset = dev-assets;
   while (asset  asset-name) {
   asset-handler(dev, AE_REMOVED, asset);
   asset++;
   }

It would be slightly safer to iterate in reverse order.

 ...
 
 
 board file:
 
   static struct regulator myreg = {
   .name = mydevice-regulator,
   };
 
   static struct device_asset mydevice_assets[] = {
   {
   .name = mydevice-regulator,
   .handler = regulator_default_asset_handler,
   },
   { }
   };
 
   static struct platform_device mydevice = {
   ...
   .dev = {
   .assets = mydevice_assets,
   },
   ...
   };
 
 
 regulator code:
 
 int regulator_default_asset_handler(struct device *dev_owner, enum 
 asset_event asset_event, struct device_asset *asset)
 {
   struct regulator **reg = asset-asset;
   int n;
 
   switch (asset_event) {
   case AE_PROBED:
   *reg = regulator_get(dev_owner, asset-name);
   if (IS_ERR(*reg))
   return *reg;

PTR_ERR.

   n = regulator_enable(*reg);
   if (n  0)
   regulator_put(*reg);
   return n;
 
   case AE_REMOVED:
   regulator_put(*reg);
   break;
   }
 
   return 0;
 }
 EXPORT_SYMBOL_GPL(regulator_default_asset_handler);
 
 
 The subsystems that can expect to get used (clock...) might each want to 
 define a default handler like the one for regulator.  That'll be an end 
 to the code duplication issue.  The user can still do his own handler if 
 he wants.
 
 I put a name field in so we can use regulator_get() nicely, we don't 
 need access to the object pointer or that it exists at boardfile-time 
 that way either.  But I can see it's arguable.

It hadn't occurred to me, but it seems like a good idea.

Yes, overall this is almost exactly what I had in mind.

  Throwing out the path stuff and limiting this to platform_device means
  you cannot bind to dynamically created objects like hub or anything
  downstream of a hub.  So Felipe's identification of the hub as the
  happening place to do this is out of luck.
 
  Greg pointed out that this could be useful for arbitrary devices, not
  just platform devices, so it could be applied to dynamically created
  objects.
 
 Well that is cool, but to exploit that in the dynamic object case 
 arrangements for identifying the appropriate object has appeared are 
 needed.

For example, this scheme wouldn't lend itself easily to associating the
regulator with the particular root-hub port that the LAN95xx is
attached to.  I can't think of any reasonable way to do that other than
the approaches we have already considered.

  We have a nice array of platform_devices nicely there in the 
 board file we can attach assets to like pdev[1].dev.assets = xxx; but 
 that's not there in dynamic device case.  Anyway this sounds like what 
 we're discussing can be well worth establishing and might lead to that 
 later.

Agreed.

Alan Stern

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  

Re: [RFC PATCH 1/5] drivers : introduce device_path api

2012-11-27 Thread Ming Lei
On Wed, Nov 28, 2012 at 1:55 AM, Andy Green andy.gr...@linaro.org wrote:
 On 11/28/2012 01:22 AM, the mail apparently from Ming Lei included:

 On Wed, Nov 28, 2012 at 12:30 AM, Alan Stern st...@rowland.harvard.edu
 wrote:

 On Tue, 27 Nov 2012, Ming Lei wrote:

 IMO, all matches mean the devices are inside the ehci-omap bus, so
 the direct/simple way is to enable/disable the regulators in the probe()
 and
 remove() of ehci-omap controller driver.


 When this discussion started, Felipe argued against such an approach.
 He pointed out that the same chip could be used in other platforms, and
 then the code added to ehci-omap.c would have to be duplicated in
 several other places.


  From Andy's implementation, the 'general' code is to enable/disable
 the regulators(patch 3/5), I am wondering if it is general enough, so the
 'duplicated' code are just several lines of
 regulator_enable/regulator_disable,
 which should be implemented in many platform drivers.


 Fair enough; my main interest was in the device path side when writing the
 patches.  I stuck enough in one place to confirm the series works on Panda
 case for power control.  So long as it doesn't get obsessed with just
 regulators some common implementation that seems to be discussed will be
 better.

 (BTW let me take this opportunity to thank you for your great urbs-in-flight
 limiting patch on smsc95xx a while back)


 Also from my intuition, power domain should be involved in the problem,
 because these hard-wired and self-powered USB devices should have
 its own power domains, and the ehci-omap driver may enable/disable
 these power domains before adding the bus.


 I don't know enough to have an opinion, but the arrangement on Panda is
 literally a linear regulator is being controlled by a gpio, which fits into
 struct regulator model.  What else would a power domain for that buy us?

One problem is that you are addressing to, another is that we may extend
Tianyu's per port power off/on mechanism into non-acpi world.

Considered that our discussion has been extended to general case instead
of pandaboard only, there might be several bus devices which have different
power control method, which should be the idea of power domain.

I have a draft idea and let me describe it by a pseudo_patch, see below:

diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
index ac17a7c..c97538f 100644
--- a/drivers/usb/host/ehci-omap.c
+++ b/drivers/usb/host/ehci-omap.c
@@ -220,6 +220,11 @@ static int ehci_hcd_omap_probe(struct
platform_device *pdev)
goto err_io;
}

+   list_for_each_entry(ppd, pdata, port_power_domain) {
+   usb_unregister_port_pm_domain(hcd-self, ppd);
+   ppd-port_power.power_on(ppd, on)
+   }
+
hcd-rsrc_start = res-start;
hcd-rsrc_len = resource_size(res);
hcd-regs = regs;
@@ -290,6 +295,11 @@ static int ehci_hcd_omap_remove(struct
platform_device *pdev)
struct usb_hcd *hcd = dev_get_drvdata(dev);
struct ehci_hcd_omap_platform_data *pdata   = dev-platform_data;

+   list_for_each_entry(ppd, pdata, port_power_domain) {
+   usb_unregister_port_pm_domain(hcd-self, ppd);
+   ppd-port_power.power_off(ppd, on)
+   }
+
usb_remove_hcd(hcd);
disable_put_regulator(dev-platform_data);
iounmap(hcd-regs);
diff --git a/include/linux/platform_data/usb-omap.h
b/include/linux/platform_data/usb-omap.h
index 8570bcf..30516c9 100644
--- a/include/linux/platform_data/usb-omap.h
+++ b/include/linux/platform_data/usb-omap.h
@@ -47,6 +47,8 @@ struct ehci_hcd_omap_platform_data {
int reset_gpio_port[OMAP3_HS_USB_PORTS];
struct regulator*regulator[OMAP3_HS_USB_PORTS];
unsignedphy_reset:1;
+
+   struct list_headport_power_domain;
 };

 struct ohci_hcd_omap_platform_data {
diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index 608050b..89c31c0 100644
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -448,6 +448,30 @@ extern void usb_disconnect(struct usb_device **);
 extern int usb_get_configuration(struct usb_device *dev);
 extern void usb_destroy_configuration(struct usb_device *dev);

+/*
+ * Only apply in hardwired self-powered devices in bus
+ */
+struct port_power_domain {
+   struct usb_bus *bus;
+   /*
+* physical port location in which the power domain provides power on 
it,
+* the first number is the root hub port, and the second number is the
+* port number on the second layer hub, ...
+*/
+   char port_info[32]; /*N-N-N...*/
+
+   /*
+* struct power_domain should be generic power thing, and should be
+* defined in power core
+*/
+   struct power_domain port_power;
+};
+
+extern int usb_register_port_pm_domain(struct usb_bus *bus, 

Re: [RFC PATCH 1/5] drivers : introduce device_path api

2012-11-27 Thread Ming Lei
On Wed, Nov 28, 2012 at 7:06 AM, Ming Lei tom.leim...@gmail.com wrote:
 Also from my intuition, power domain should be involved in the problem,
 because these hard-wired and self-powered USB devices should have
 its own power domains, and the ehci-omap driver may enable/disable
 these power domains before adding the bus.


 I don't know enough to have an opinion, but the arrangement on Panda is
 literally a linear regulator is being controlled by a gpio, which fits into
 struct regulator model.  What else would a power domain for that buy us?

 One problem is that you are addressing to, another is that we may extend
 Tianyu's per port power off/on mechanism into non-acpi world.

 Considered that our discussion has been extended to general case instead
 of pandaboard only, there might be several bus devices which have different
 power control method, which should be the idea of power domain.

 I have a draft idea and let me describe it by a pseudo_patch, see below:

Sorry, looks sending it too quick, the below pseudo_patch may be
more readable about the idea.

diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
index ac17a7c..c187a11 100644
--- a/drivers/usb/host/ehci-omap.c
+++ b/drivers/usb/host/ehci-omap.c
@@ -184,6 +184,7 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev)
int irq;
int i;
charsupply[7];
+   struct port_power_domain*ppd;

if (usb_disabled())
return -ENODEV;
@@ -220,6 +221,17 @@ static int ehci_hcd_omap_probe(struct
platform_device *pdev)
goto err_io;
}

+   /*
+* register usb per port power domain and enable power on
+* this port, to which only hardwired and self-powered device
+* attached. And the platform code should provide the
+* port power domain list to the usb host controller driver.
+*/
+   list_for_each_entry(ppd, pdata-port_power_domain, list) {
+   usb_register_port_pm_domain(hcd-self, ppd);
+   usb_enable_port_pm_domain(hcd-self, ppd);
+   }
+
hcd-rsrc_start = res-start;
hcd-rsrc_len = resource_size(res);
hcd-regs = regs;
@@ -289,6 +301,12 @@ static int ehci_hcd_omap_remove(struct
platform_device *pdev)
struct device *dev  = pdev-dev;
struct usb_hcd *hcd = dev_get_drvdata(dev);
struct ehci_hcd_omap_platform_data *pdata   = dev-platform_data;
+   struct port_power_domain*ppd;
+
+   list_for_each_entry(ppd, pdata-port_power_domain, list) {
+   usb_disable_port_pm_domain(hcd-self, ppd);
+   usb_unregister_port_pm_domain(hcd-self, ppd);
+   }

usb_remove_hcd(hcd);
disable_put_regulator(dev-platform_data);
diff --git a/include/linux/platform_data/usb-omap.h
b/include/linux/platform_data/usb-omap.h
index 8570bcf..30516c9 100644
--- a/include/linux/platform_data/usb-omap.h
+++ b/include/linux/platform_data/usb-omap.h
@@ -47,6 +47,8 @@ struct ehci_hcd_omap_platform_data {
int reset_gpio_port[OMAP3_HS_USB_PORTS];
struct regulator*regulator[OMAP3_HS_USB_PORTS];
unsignedphy_reset:1;
+
+   struct list_headport_power_domain;
 };

 struct ohci_hcd_omap_platform_data {
diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index 608050b..6b86d01 100644
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -448,6 +448,39 @@ extern void usb_disconnect(struct usb_device **);
 extern int usb_get_configuration(struct usb_device *dev);
 extern void usb_destroy_configuration(struct usb_device *dev);

+/*
+ * Only used for describing power domain which provide power to
+ * hardwired self-powered devices
+ */
+struct port_power_domain {
+   struct list_head list;
+   struct usb_bus *bus;
+
+   /*
+* physical port path, and the power domain of 'port_power' provides
+* power on the device attatched to the last non-zero port(Pn-1) of
+* the n-1 tier hub, the first number(P1) is the root hub port in
+* the path, and the second number(P2) is the port number on the
+* second tier hub, ..., until the last number Pn which is zero always.
+*/
+   char port_path[32]; /* P1-P2-..Pn-1-Pn */
+
+   /*
+* struct power_domain should be generic power thing, and should be
+* defined in device power core, maybe it can reuse some kind of
+* current power domain structure.
+*
+* Many ports can share one same power domain, so make the below field
+* as pointer.
+*/
+   struct power_domain *port_power;
+};
+
+extern int usb_register_port_pm_domain(struct usb_bus *bus, 

Re: [RFC PATCH 1/5] drivers : introduce device_path api

2012-11-27 Thread Andy Green

On 11/28/2012 04:10 AM, the mail apparently from Alan Stern included:

On Wed, 28 Nov 2012, Andy Green wrote:


OK.  So I try to sketch it out iteractively to try to get in sync:

device.h:

enum asset_event {
AE_PROBED,
AE_REMOVED
};

struct device_asset {
char *name; /* name of regulator, clock, etc */
void *asset; /* regulator, clock, etc */
int (*handler)(struct device *dev_owner, enum asset_event 
asset_event,
struct device_asset *asset);
};


Another possibility is to have two handlers: one for pre-probe and the
other for post-remove.  Then of course asset_event wouldn't be needed.
Linus tends to prefer this strategy -- separate functions for separate
events.  That's why struct dev_pm_ops has so many method pointers.


OK.

I wonder if this needs extending to PM ops passing in to the assets. 
Regulator is usually self-sufficient but sometimes clocks at least need 
meddling in suspend paths.


Maybe it's enough instead to offer a standalone api for drivers that 
want to meddle with assets, like enable / disable an asset clock...


void *device_find_asset(struct device *device, const char *name);

...if it wants to touch anything like that it needs to mandate a 
nonambiguous name for the asset like reg-mydriver-ehci-omap.0.


This is also handy since the driver can then adapt around absence or 
presence of optional assets if it wants.



struct device {
...
struct device_asset *assets;


Or a list instead of a NULL-terminated array.  It depends on whether
people will want to add or remove assets dynamically.  For now an array
would be fine.


OK.


...
};


drivers/base/dd.c | really_probe():

...
struct device_asset *asset;
...
asset = dev-assets;
while (asset  asset-name) {


Maybe it would be better to test asset-handler.  Some assets might not
need names.


Good point.


if (asset-handler(dev, AE_PROBED, asset)) {
/* clean up and bail */
}
asset++;
}

/* do probe */
...


drivers/base/dd.c | __device_release_driver:  (is this really the best
place to oppose probe()?)


The right place is just after the remove method is called, so yes.


...
struct device_asset *asset;
...

/* call device -remove() */
...
asset = dev-assets;
while (asset  asset-name) {
asset-handler(dev, AE_REMOVED, asset);
asset++;
}


It would be slightly safer to iterate in reverse order.


Good point.


...


board file:

static struct regulator myreg = {
.name = mydevice-regulator,
};

static struct device_asset mydevice_assets[] = {
{
.name = mydevice-regulator,
.handler = regulator_default_asset_handler,
},
{ }
};

static struct platform_device mydevice = {
...
.dev = {
.assets = mydevice_assets,
},
...
};


regulator code:

int regulator_default_asset_handler(struct device *dev_owner, enum
asset_event asset_event, struct device_asset *asset)
{
struct regulator **reg = asset-asset;
int n;

switch (asset_event) {
case AE_PROBED:
*reg = regulator_get(dev_owner, asset-name);
if (IS_ERR(*reg))
return *reg;


PTR_ERR.


Right.

I'll offer a series with these adaptations shortly.

-Andy


n = regulator_enable(*reg);
if (n  0)
regulator_put(*reg);
return n;

case AE_REMOVED:
regulator_put(*reg);
break;
}

return 0;
}
EXPORT_SYMBOL_GPL(regulator_default_asset_handler);


The subsystems that can expect to get used (clock...) might each want to
define a default handler like the one for regulator.  That'll be an end
to the code duplication issue.  The user can still do his own handler if
he wants.

I put a name field in so we can use regulator_get() nicely, we don't
need access to the object pointer or that it exists at boardfile-time
that way either.  But I can see it's arguable.


It hadn't occurred to me, but it seems like a good idea.

Yes, overall this is almost exactly what I had in mind.


Throwing out the path stuff and limiting this to platform_device means
you cannot bind to dynamically created objects like hub or anything
downstream of a hub.  So Felipe's identification of the hub as the
happening place to do this is out of luck.


Greg pointed out that this could be useful for arbitrary devices, not
just platform devices, so it could be applied to dynamically created
objects.


Well that is cool, but to exploit that in the dynamic object case

[RFC PATCH 1/5] drivers : introduce device_path api

2012-11-26 Thread Andy Green
This adds a small optional API into drivers/base which deals with generating,
matching and registration of wildcard device paths.

From a struct device * you can generate a string like

 /platform/usbhs_omap/ehci-omap.0/usb1/1-1

which enapsulates the path of the device's connection to the board.

These can be used to match up other assets, for example struct regulators,
that have been registed elsewhere with a device instance that is probed
asynchronously from the other board assets.

If your device is on a bus, as it probably is, the device path will feature
redundant bus indexes that do not contain information about the connectivity.

For example if more than one driver can generate devices on the same bus,
then the ordering of device probing will change the path, despite the
connectivity remains the same.

For that reason, to get a deterministic path for matching, wildcards are
allowed.  If your target device has the path

 /platform/usbhs_omap/ehci-omap.0/usb1/1-1

generated, in the asset you wish to match with it you can instead use

/platform/usbhs_omap/ehci-omap.0/usb*/*-1

in order to only leave the useful, invariant parts of the path to match
against.

To avoid having to adapt every kind of search by string api to also use
the wildcards, the api takes the approach you can register your wildcard,
and if a matching path is generated for a device, the wildcard itself is
handed back as the device path instead.

So if your board code or a (not yet done) DT binding registers this wildcard

 /platform/usbhs_omap/ehci-omap.0/usb*

and the usb hub driver asks to generate its device path

device_path_generate(dev, name, sizeof name);

that is actually

 /platform/usbhs_omap/ehci-omap.0/usb1

then what will be returned is

 /platform/usbhs_omap/ehci-omap.0/usb*

providing the same literal string for ehci-omap.0 hub no matter how many\
usb buses have been registered before.

This wildcard string can then be matched to regulators or other string-
searchable assets intended for the device on that hardware path.

Signed-off-by: Andy Green andy.gr...@linaro.org
---
 drivers/base/Kconfig   |6 ++
 drivers/base/Makefile  |2 +
 drivers/base/core.c|2 +
 drivers/base/path.c|  181 
 include/linux/device.h |   12 +++
 5 files changed, 203 insertions(+)
 create mode 100644 drivers/base/path.c

diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
index b34b5cd..3324a55 100644
--- a/drivers/base/Kconfig
+++ b/drivers/base/Kconfig
@@ -282,4 +282,10 @@ config CMA_AREAS
 
 endif
 
+config DEVICEPATH
+   bool Device path api
+   default n
+   help
+ Include dynamicly probed path matching API
+
 endmenu
diff --git a/drivers/base/Makefile b/drivers/base/Makefile
index 5aa2d70..b8d5723 100644
--- a/drivers/base/Makefile
+++ b/drivers/base/Makefile
@@ -22,5 +22,7 @@ obj-$(CONFIG_SYS_HYPERVISOR) += hypervisor.o
 obj-$(CONFIG_REGMAP)   += regmap/
 obj-$(CONFIG_SOC_BUS) += soc.o
 
+obj-$(CONFIG_DEVICEPATH) += path.o
+
 ccflags-$(CONFIG_DEBUG_DRIVER) := -DDEBUG
 
diff --git a/drivers/base/core.c b/drivers/base/core.c
index abea76c..cc0ba02 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -1368,6 +1368,8 @@ int __init devices_init(void)
if (!sysfs_dev_char_kobj)
goto char_kobj_err;
 
+   device_path_init();
+
return 0;
 
  char_kobj_err:
diff --git a/drivers/base/path.c b/drivers/base/path.c
new file mode 100644
index 000..384e792
--- /dev/null
+++ b/drivers/base/path.c
@@ -0,0 +1,181 @@
+/*
+ * drivers/base/path.c - device_path apis for matching dynamic / variable
+ *  device paths on buses like usb / mmc to wildcard constants from
+ *  platform or DT
+ *
+ * Copyright (c) 2012 Linaro, LTD
+ * Author: Andy Green andy.gr...@linaro.org
+ *
+ * This file is released under the GPLv2
+ *
+ */
+
+#include linux/device.h
+#include linux/err.h
+#include linux/list.h
+#include linux/mutex.h
+#include linux/string.h
+#include linux/export.h
+#include linux/slab.h
+
+struct device_path {
+   char *path;
+   struct list_head list;
+};
+
+struct device_path list;
+DEFINE_MUTEX(lock);
+
+static int device_path_compare_wildcard(const char *awc, const char *b)
+{
+   while (*awc  *b) {
+   if (*awc == '*') {
+   awc++;
+   /* wildcard disallowed from extening past / */
+   while (*b  *b != *awc  *b != '/')
+   b++;
+   }
+   if (*awc != *b)
+   return -ENOENT;
+   if (!*awc)
+   return 0;
+   awc++;
+   b++;
+   }
+
+   if (!*awc  !*b)
+   return 0;
+
+   return -ENOENT;
+}
+
+static const char *device_path_find_wildcard(const char *device_path)
+{
+   struct device_path *dp;
+
+   mutex_lock(lock);
+   list_for_each_entry(dp, list.list, list) {
+  

Re: [RFC PATCH 1/5] drivers : introduce device_path api

2012-11-26 Thread Alan Stern
On Mon, 26 Nov 2012, Andy Green wrote:

 This adds a small optional API into drivers/base which deals with generating,
 matching and registration of wildcard device paths.
 
 From a struct device * you can generate a string like
 
  /platform/usbhs_omap/ehci-omap.0/usb1/1-1
 
 which enapsulates the path of the device's connection to the board.
 
 These can be used to match up other assets, for example struct regulators,
 that have been registed elsewhere with a device instance that is probed
 asynchronously from the other board assets.
 
 If your device is on a bus, as it probably is, the device path will feature
 redundant bus indexes that do not contain information about the connectivity.
 
 For example if more than one driver can generate devices on the same bus,
 then the ordering of device probing will change the path, despite the
 connectivity remains the same.
 
 For that reason, to get a deterministic path for matching, wildcards are
 allowed.  If your target device has the path
 
  /platform/usbhs_omap/ehci-omap.0/usb1/1-1
 
 generated, in the asset you wish to match with it you can instead use
 
 /platform/usbhs_omap/ehci-omap.0/usb*/*-1
 
 in order to only leave the useful, invariant parts of the path to match
 against.

Have you considered limiting these wildcards in various useful ways?  
In this example, the wildcard must match a string of decimal digits.  
(Furthermore the two wildcard strings will always match each other, 
although it's probably not necessary to add this sort of constraint.)

I don't know what sort of matches people will want in the future.  
Perhaps one for hex digits, or one for arbitrary text with the
exception of a few characters (such as '/' but perhaps others too).

To do what you want for now, the match should be restricted to digits.

 To avoid having to adapt every kind of search by string api to also use
 the wildcards, the api takes the approach you can register your wildcard,
 and if a matching path is generated for a device, the wildcard itself is
 handed back as the device path instead.
 
 So if your board code or a (not yet done) DT binding registers this wildcard
 
  /platform/usbhs_omap/ehci-omap.0/usb*
 
 and the usb hub driver asks to generate its device path
 
   device_path_generate(dev, name, sizeof name);
 
 that is actually
 
  /platform/usbhs_omap/ehci-omap.0/usb1
 
 then what will be returned is
 
  /platform/usbhs_omap/ehci-omap.0/usb*
 
 providing the same literal string for ehci-omap.0 hub no matter how many\
 usb buses have been registered before.
 
 This wildcard string can then be matched to regulators or other string-
 searchable assets intended for the device on that hardware path.

That's not how I would do it, at least, not for this application.  I
would register tuples containing a name, a pointer, and two callback
routines.  For example,

(/platform/usbhs_omap/ehci-omap.0/usb*, omap_vhub_device,
turn_on_regulator, turn_off_regulator)

The when a device is registered whose path matches the string in such a
tuple, the device core would know to invoke the first callback.  The
arguments would be a pointer to the device being registered and the
pointer in the tuple.  Similarly, the device core would invoke the
second callback when the device is unregistered.

There doesn't have to be anything in this scheme that's specific to 
hubs or even to USB.  In particular, no changes to the hub driver would 
be needed.

Alan Stern

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH 1/5] drivers : introduce device_path api

2012-11-26 Thread Greg KH
On Mon, Nov 26, 2012 at 12:45:34PM +, Andy Green wrote:
 +struct device_path list;
 +DEFINE_MUTEX(lock);

Those are some very descriptive global variables you just created :(

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH 1/5] drivers : introduce device_path api

2012-11-26 Thread Greg KH
On Mon, Nov 26, 2012 at 12:45:34PM +, Andy Green wrote:
 This adds a small optional API into drivers/base which deals with generating,
 matching and registration of wildcard device paths.
 
 From a struct device * you can generate a string like
 
  /platform/usbhs_omap/ehci-omap.0/usb1/1-1
 
 which enapsulates the path of the device's connection to the board.
 
 These can be used to match up other assets, for example struct regulators,
 that have been registed elsewhere with a device instance that is probed
 asynchronously from the other board assets.
 
 If your device is on a bus, as it probably is, the device path will feature
 redundant bus indexes that do not contain information about the connectivity.

Huh?  A bus index does show the connectivity, well, one type of
connectivity, but perhaps it's not the one _you_ happen to want at the
moment.  Which is fine, but I don't see why you want to try to figure
this out using the device path in the first place, surely you have some
other way that the hardware can describe itself to the kernel as to
where it needs to be hooked up to?

 For example if more than one driver can generate devices on the same bus,
 then the ordering of device probing will change the path, despite the
 connectivity remains the same.

That's an expected thing, I don't see the issue here.

 For that reason, to get a deterministic path for matching, wildcards are
 allowed.  If your target device has the path

Wait, no, why would you want a deterministic path and have that
hard-coded into the kernel here?  You can't rely on that any more than
userspace can, so let's not start making the mistake that lots of
userspace programmers originally did when they started using sysfs
please.  We have learned from our past mistakes.

  /platform/usbhs_omap/ehci-omap.0/usb1/1-1
 
 generated, in the asset you wish to match with it you can instead use
 
 /platform/usbhs_omap/ehci-omap.0/usb*/*-1
 
 in order to only leave the useful, invariant parts of the path to match
 against.
 
 To avoid having to adapt every kind of search by string api to also use
 the wildcards, the api takes the approach you can register your wildcard,
 and if a matching path is generated for a device, the wildcard itself is
 handed back as the device path instead.
 
 So if your board code or a (not yet done) DT binding registers this wildcard
 
  /platform/usbhs_omap/ehci-omap.0/usb*

Device tree lists sysfs paths in it's descriptions?  That's news to me.

 and the usb hub driver asks to generate its device path
 
   device_path_generate(dev, name, sizeof name);
 
 that is actually
 
  /platform/usbhs_omap/ehci-omap.0/usb1
 
 then what will be returned is
 
  /platform/usbhs_omap/ehci-omap.0/usb*
 
 providing the same literal string for ehci-omap.0 hub no matter how many\
 usb buses have been registered before.
 
 This wildcard string can then be matched to regulators or other string-
 searchable assets intended for the device on that hardware path.

Ah, here's the root of your problem, right?  You need a way for your
hardware to tell the kernel that you have a regulator attached to a
specific device?  Using the device path and hard-coding it into the
kernel is not the proper way to solve this, sorry.  Use some other
unique way to describe the hardware, surely the hardware designers
couldn't have been that foolish not to provide this [1]?

thanks,

greg k-h

[1] Yeah, I know I'm being hopeful here, and probably wrong, but then
you need to push back.  We are not helpless here.
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH 1/5] drivers : introduce device_path api

2012-11-26 Thread Greg KH
On Mon, Nov 26, 2012 at 11:22:06AM -0800, Greg KH wrote:
 On Mon, Nov 26, 2012 at 12:45:34PM +, Andy Green wrote:
  This adds a small optional API into drivers/base which deals with 
  generating,
  matching and registration of wildcard device paths.
  
  From a struct device * you can generate a string like
  
   /platform/usbhs_omap/ehci-omap.0/usb1/1-1
  
  which enapsulates the path of the device's connection to the board.
  
  These can be used to match up other assets, for example struct regulators,
  that have been registed elsewhere with a device instance that is probed
  asynchronously from the other board assets.
  
  If your device is on a bus, as it probably is, the device path will feature
  redundant bus indexes that do not contain information about the 
  connectivity.
 
 Huh?  A bus index does show the connectivity, well, one type of
 connectivity, but perhaps it's not the one _you_ happen to want at the
 moment.  Which is fine, but I don't see why you want to try to figure
 this out using the device path in the first place, surely you have some
 other way that the hardware can describe itself to the kernel as to
 where it needs to be hooked up to?
 
  For example if more than one driver can generate devices on the same bus,
  then the ordering of device probing will change the path, despite the
  connectivity remains the same.
 
 That's an expected thing, I don't see the issue here.
 
  For that reason, to get a deterministic path for matching, wildcards are
  allowed.  If your target device has the path
 
 Wait, no, why would you want a deterministic path and have that
 hard-coded into the kernel here?  You can't rely on that any more than
 userspace can, so let's not start making the mistake that lots of
 userspace programmers originally did when they started using sysfs
 please.  We have learned from our past mistakes.
 
   /platform/usbhs_omap/ehci-omap.0/usb1/1-1

Oh, and further proof of this, there are patches floating around to drop
the platform name from the sys/drivers/ tree, so your driver just
broke if that goes through, showing you really don't want to be
hard-coding sysfs paths in any type of logic.

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH 1/5] drivers : introduce device_path api

2012-11-26 Thread Alan Stern
On Mon, 26 Nov 2012, Greg KH wrote:

 Ah, here's the root of your problem, right?  You need a way for your
 hardware to tell the kernel that you have a regulator attached to a
 specific device?  Using the device path and hard-coding it into the
 kernel is not the proper way to solve this, sorry.  Use some other
 unique way to describe the hardware, surely the hardware designers
 couldn't have been that foolish not to provide this [1]?

As far as I know, the kernel has no other way to describe devices.

What about using partial matches?  In this example, instead of doing a 
wildcard match against

/platform/usbhs_omap/ehci-omap.0/usb*

(which would fail if the platform part of the path changes), suppose 
the string ehci-omap.0/usb* could be associated with the usbhs_omap 
component somehow.  Or even better, the string usb* could be 
associated with the ehci-omap.0 device.

Then the path-matching code could restrict its attention to that part
of the path and to devices below the specified one.  Naming changes
wouldn't be an issue, because the changes themselves would be made in
the same source file that contains the partial path string.


On the other hand, this may be way more general than we really need.  
For this particular case, all we need to do is take special action the
first time any device is registered below the ehci-omap.0 platform
device.  There ought to be a more direct way to accomplish this,
without using string pattern-matching or sysfs pathnames (and without 
adding overhead every time a device is added or deleted).

I don't know if such an approach would be sufficiently general for 
future requirements, but it would solve the problem at hand.

Alan Stern

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH 1/5] drivers : introduce device_path api

2012-11-26 Thread Andy Green

On 11/27/2012 03:12 AM, the mail apparently from Alan Stern included:

On Mon, 26 Nov 2012, Andy Green wrote:


This adds a small optional API into drivers/base which deals with generating,
matching and registration of wildcard device paths.

From a struct device * you can generate a string like

  /platform/usbhs_omap/ehci-omap.0/usb1/1-1

which enapsulates the path of the device's connection to the board.

These can be used to match up other assets, for example struct regulators,
that have been registed elsewhere with a device instance that is probed
asynchronously from the other board assets.

If your device is on a bus, as it probably is, the device path will feature
redundant bus indexes that do not contain information about the connectivity.

For example if more than one driver can generate devices on the same bus,
then the ordering of device probing will change the path, despite the
connectivity remains the same.

For that reason, to get a deterministic path for matching, wildcards are
allowed.  If your target device has the path

  /platform/usbhs_omap/ehci-omap.0/usb1/1-1

generated, in the asset you wish to match with it you can instead use

/platform/usbhs_omap/ehci-omap.0/usb*/*-1

in order to only leave the useful, invariant parts of the path to match
against.


Have you considered limiting these wildcards in various useful ways?
In this example, the wildcard must match a string of decimal digits.
(Furthermore the two wildcard strings will always match each other,
although it's probably not necessary to add this sort of constraint.)

I don't know what sort of matches people will want in the future.
Perhaps one for hex digits, or one for arbitrary text with the
exception of a few characters (such as '/' but perhaps others too).

To do what you want for now, the match should be restricted to digits.


I'm not sure what we'd use that for... it doesn't seem the biggest 
problem we have at the moment ^^



To avoid having to adapt every kind of search by string api to also use
the wildcards, the api takes the approach you can register your wildcard,
and if a matching path is generated for a device, the wildcard itself is
handed back as the device path instead.

So if your board code or a (not yet done) DT binding registers this wildcard

  /platform/usbhs_omap/ehci-omap.0/usb*

and the usb hub driver asks to generate its device path

device_path_generate(dev, name, sizeof name);

that is actually

  /platform/usbhs_omap/ehci-omap.0/usb1

then what will be returned is

  /platform/usbhs_omap/ehci-omap.0/usb*

providing the same literal string for ehci-omap.0 hub no matter how many\
usb buses have been registered before.

This wildcard string can then be matched to regulators or other string-
searchable assets intended for the device on that hardware path.


That's not how I would do it, at least, not for this application.  I
would register tuples containing a name, a pointer, and two callback
routines.  For example,

(/platform/usbhs_omap/ehci-omap.0/usb*, omap_vhub_device,
turn_on_regulator, turn_off_regulator)

The when a device is registered whose path matches the string in such a
tuple, the device core would know to invoke the first callback.  The
arguments would be a pointer to the device being registered and the
pointer in the tuple.  Similarly, the device core would invoke the
second callback when the device is unregistered.

There doesn't have to be anything in this scheme that's specific to
hubs or even to USB.  In particular, no changes to the hub driver would
be needed.


By just using paths, it's not restricted to regulators or binary 
operations on them but anything that needs a floating binding to 
another named kernel object can leverage it.


-Andy

--
Andy Green | TI Landing Team Leader
Linaro.org │ Open source software for ARM SoCs | Follow Linaro
http://facebook.com/pages/Linaro/155974581091106  - 
http://twitter.com/#!/linaroorg - http://linaro.org/linaro-blog

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH 1/5] drivers : introduce device_path api

2012-11-26 Thread Andy Green

On 11/27/2012 03:16 AM, the mail apparently from Greg KH included:

On Mon, Nov 26, 2012 at 12:45:34PM +, Andy Green wrote:

+struct device_path list;
+DEFINE_MUTEX(lock);


Those are some very descriptive global variables you just created :(



I guess I can add the static if that will heal the emotional damage I 
caused.


-Andy

--
Andy Green | TI Landing Team Leader
Linaro.org │ Open source software for ARM SoCs | Follow Linaro
http://facebook.com/pages/Linaro/155974581091106  - 
http://twitter.com/#!/linaroorg - http://linaro.org/linaro-blog

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH 1/5] drivers : introduce device_path api

2012-11-26 Thread Andy Green

On 11/27/2012 03:22 AM, the mail apparently from Greg KH included:

On Mon, Nov 26, 2012 at 12:45:34PM +, Andy Green wrote:

This adds a small optional API into drivers/base which deals with generating,
matching and registration of wildcard device paths.

From a struct device * you can generate a string like

  /platform/usbhs_omap/ehci-omap.0/usb1/1-1

which enapsulates the path of the device's connection to the board.

These can be used to match up other assets, for example struct regulators,
that have been registed elsewhere with a device instance that is probed
asynchronously from the other board assets.

If your device is on a bus, as it probably is, the device path will feature
redundant bus indexes that do not contain information about the connectivity.


Huh?  A bus index does show the connectivity, well, one type of
connectivity, but perhaps it's not the one _you_ happen to want at the
moment.  Which is fine, but I don't see why you want to try to figure
this out using the device path in the first place, surely you have some
other way that the hardware can describe itself to the kernel as to
where it needs to be hooked up to?


The bus index is like a counter, it shows logical connectivity inside 
Linux that isn't repeatable and doesn't reflect hardware routing 
information directly.



For example if more than one driver can generate devices on the same bus,
then the ordering of device probing will change the path, despite the
connectivity remains the same.


That's an expected thing, I don't see the issue here.


Alan brought up in a thread here the last days, wouldn't it be nice if 
we could arbitrarily bind regulators to asynchronously probed objects, 
and after discussion proposed this wildcard matching scheme to mask 
these generated bus numbers.



For that reason, to get a deterministic path for matching, wildcards are
allowed.  If your target device has the path


Wait, no, why would you want a deterministic path and have that
hard-coded into the kernel here?  You can't rely on that any more than
userspace can, so let's not start making the mistake that lots of
userspace programmers originally did when they started using sysfs
please.  We have learned from our past mistakes.


I guess that is a fair point.  I was going to say that it's no different 
than using any kernel API in code, which history proves is very mutable; 
people deal with it by changing the usages as they change the API 
definition.  But it's complicated a bit by DTs meant to be more stable 
and these paths would turn up in the DT.


In platform case though, a heuristic that leaving off the initial / 
and allowing matches anywhere along the path then to the end would get 
around it.



  /platform/usbhs_omap/ehci-omap.0/usb1/1-1

generated, in the asset you wish to match with it you can instead use

/platform/usbhs_omap/ehci-omap.0/usb*/*-1

in order to only leave the useful, invariant parts of the path to match
against.

To avoid having to adapt every kind of search by string api to also use
the wildcards, the api takes the approach you can register your wildcard,
and if a matching path is generated for a device, the wildcard itself is
handed back as the device path instead.

So if your board code or a (not yet done) DT binding registers this wildcard

  /platform/usbhs_omap/ehci-omap.0/usb*


Device tree lists sysfs paths in it's descriptions?  That's news to me.


It does not say that...


and the usb hub driver asks to generate its device path

device_path_generate(dev, name, sizeof name);

that is actually

  /platform/usbhs_omap/ehci-omap.0/usb1

then what will be returned is

  /platform/usbhs_omap/ehci-omap.0/usb*

providing the same literal string for ehci-omap.0 hub no matter how many\
usb buses have been registered before.

This wildcard string can then be matched to regulators or other string-
searchable assets intended for the device on that hardware path.


Ah, here's the root of your problem, right?  You need a way for your
hardware to tell the kernel that you have a regulator attached to a
specific device?  Using the device path and hard-coding it into the
kernel is not the proper way to solve this, sorry.  Use some other
unique way to describe the hardware, surely the hardware designers
couldn't have been that foolish not to provide this [1]?

thanks,

greg k-h

[1] Yeah, I know I'm being hopeful here, and probably wrong, but then
 you need to push back.  We are not helpless here.


Specific hardware information is something that's kept hidden away and 
private in the driver for good reasons.


We could add elective, additional information at the driver for every 
physical interface it represented and match that way.  But I am not sure 
the effort involved is repaid by the relatively few instances that need 
what is basically asynchronously probed platform_data.


-Andy

--
Andy Green | TI Landing Team Leader
Linaro.org │ Open source software for ARM SoCs | Follow Linaro

Re: [RFC PATCH 1/5] drivers : introduce device_path api

2012-11-26 Thread Andy Green

On 11/27/2012 05:07 AM, the mail apparently from Alan Stern included:

On Mon, 26 Nov 2012, Greg KH wrote:


Ah, here's the root of your problem, right?  You need a way for your
hardware to tell the kernel that you have a regulator attached to a
specific device?  Using the device path and hard-coding it into the
kernel is not the proper way to solve this, sorry.  Use some other
unique way to describe the hardware, surely the hardware designers
couldn't have been that foolish not to provide this [1]?


As far as I know, the kernel has no other way to describe devices.

What about using partial matches?  In this example, instead of doing a
wildcard match against

/platform/usbhs_omap/ehci-omap.0/usb*

(which would fail if the platform part of the path changes), suppose
the string ehci-omap.0/usb* could be associated with the usbhs_omap
component somehow.  Or even better, the string usb* could be
associated with the ehci-omap.0 device.

Then the path-matching code could restrict its attention to that part
of the path and to devices below the specified one.  Naming changes
wouldn't be an issue, because the changes themselves would be made in
the same source file that contains the partial path string.


Yes just dropping the starting / on the match and allowing a fragment to 
match further up the string would solve it.  ehci-omap.0 won't appear 
down any other earlier device paths than the right one.



On the other hand, this may be way more general than we really need.
For this particular case, all we need to do is take special action the
first time any device is registered below the ehci-omap.0 platform
device.  There ought to be a more direct way to accomplish this,
without using string pattern-matching or sysfs pathnames (and without
adding overhead every time a device is added or deleted).


There are no sysfs pathnames involved here at all.  Greg invented that.


I don't know if such an approach would be sufficiently general for
future requirements, but it would solve the problem at hand.


We can get a notification about device creation and do things there. 
But the cost of that is like the tuple suggestion, they'll only be able 
to do a fixed thing like operate on regulators.


Actually the targeted device may have arbitrary associated assets like 
generic GPIO to turn on a flash for built-in webcam controlled 
out-of-band.  These also can be reached by known names.  And the driver 
may wish to do things with them that are more complex than enable / 
disable or follow logical lifecycle of the hub or whatever.


However when the guidance from Greg is that we can do nothing except 
complain at hardware designers for some reason I failed to quite 
identify, I fear we are moving deckchairs on the titanic.


-Andy

--
Andy Green | TI Landing Team Leader
Linaro.org │ Open source software for ARM SoCs | Follow Linaro
http://facebook.com/pages/Linaro/155974581091106  - 
http://twitter.com/#!/linaroorg - http://linaro.org/linaro-blog

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH 1/5] drivers : introduce device_path api

2012-11-26 Thread Ming Lei
On Tue, Nov 27, 2012 at 5:07 AM, Alan Stern st...@rowland.harvard.edu wrote:
 On Mon, 26 Nov 2012, Greg KH wrote:

 Ah, here's the root of your problem, right?  You need a way for your
 hardware to tell the kernel that you have a regulator attached to a
 specific device?  Using the device path and hard-coding it into the
 kernel is not the proper way to solve this, sorry.  Use some other
 unique way to describe the hardware, surely the hardware designers
 couldn't have been that foolish not to provide this [1]?

 As far as I know, the kernel has no other way to describe devices.

 What about using partial matches?  In this example, instead of doing a
 wildcard match against

 /platform/usbhs_omap/ehci-omap.0/usb*

IMO, all matches mean the devices are inside the ehci-omap bus, so
the direct/simple way is to enable/disable the regulators in the probe() and
remove() of ehci-omap controller driver.

On the other side, both the two LAN95xx USB devices(hub + ethernet) are
simple self-powered device. Just like other self-powered devices, the power
should be provided from external world, instead of hub driver itself. And it is
doable to power on the devices before creating the specific ehci-omap usb
bus inside ehci-omap driver.

Thanks,
-- 
Ming Lei
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html