On Tue, Jun 21, 2016 at 10:11:17AM +0800, Peter Chen wrote:
> On Mon, Jun 20, 2016 at 11:16:07AM -0500, Rob Herring wrote:
> > On Mon, Jun 20, 2016 at 07:26:51PM +0800, Peter Chen wrote:
> > > On Fri, Jun 17, 2016 at 12:16:48PM -0500, Rob Herring wrote:
> > > > On Fri, Jun 17, 2016 at 5:09 AM, Peter Chen <peter.chen@xxxxxxx> wrote:
> > > > > Add binding doc for generic usb power sequence driver, and update
> > > > > generic usb device binding-doc accordingly.
[...]
> > > clocks = <&clks IMX6SX_CLK_CKO>;
> > >
> > > #address-cells = <1>;
> > > #size-cells = <0>;
> > > ethernet: asix@1 {
> > > compatible = "usbb95,1708";
> > > reg = <1>;
> > >
> > > power-sequence;
> > > reset-gpios = <&gpio4 6 GPIO_ACTIVE_LOW>; /* ethernet_rst */
> > > reset-duration-us = <15>;
> > > clocks = <&clks IMX6SX_CLK_IPG>;
> > > };
> > > };
> > > };
> > >
> > > If the node has property "power-sequence", the pwrseq core will create
> > > related platform device, and the driver under pwrseq driver will handle
> > > power sequence stuffs.
> >
> > This I have issue with. If you are creating a platform device here, you
> > are trying to work-around limitations in the linux driver model.
>
> My current solution like below, but it seems you didn't agree with that.
> I just double confirm here, if you don't, I give up the solution for
> using generic power sequence framework.
>
> In drivers/usb/core/hub.c
>
> for_each_child_of_node(parent->of_node, node) {
> hdev_pwrseq = pwrseq_alloc(node, "usb_pwrseq_generic");
> if (!IS_ERR_OR_NULL(hdev_pwrseq)) {
> pwrseq_node = kzalloc(sizeof(pwrseq_node), GFP_KERNEL);
> if (!pwrseq_node) {
> ret = -ENOMEM;
> goto err1;
> }
> /* power on sequence */
> ret = pwrseq_pre_power_on(hdev_pwrseq);
Why does this function need to do anything more than:
- Check if the child has a "power-sequence" property
- Get the "reset-gpios" GPIO
- Assert reset for specified/default time
- Deassert reset
Then continue on as normal. That seems straight-forward to me.
There is no reason you need a platform device in the mix. Perhaps trying
to move the MMC pwr-seq code is pointless as it adds needless
complexity.
[...]
> > Either
> > we need some sort of pre-probe hook to the drivers to call or each
> > parent node driver is responsible for checking and calling pwr-seq
> > functions for child nodes. e.g. The host controller calls pwr-seq for
> > the hub, the hub driver calls the power seq for the asix chip. Soon as
> > we have a case too complex for the generic pwr-seq, we're going to need
> > the pre-probe hook as I don't want to see a continual expansion of
> > generic pwr-seq binding for ever more complex cases.
> >
>
> How the driver know what it needs to handle (eg, gpio, clock) if there
> is no device for it? The most important we need to consider is which
> device owns there power sequence properties, then the corresponding
> driver can handle it.
What can be handled by is defined by presence of power-sequence
property. There can be 1 driver for the device. That is the USB hub
driver in this example. You should not have 2 "devices".
Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
