On Tue, Jun 21, 2016 at 04:26:53PM -0500, Rob Herring wrote:
> 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
>
Besides gpios, it may exist clock and regulator operation, and the
operation may be failed. I thought these operations can be easy
done belongs to a device, but now, let me try this straight-forward
way, thanks.
Peter
> 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
--
Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
