Re: [RFC/PATCH] of: of_find_node_by_name - stop dropping reference to 'from' node

[Benjamin Herrenschmidt]

On Thu, 2016-04-21 at 15:35 -0700, Frank Rowand wrote:
> No.  It is correct for of_find_by_name() to call of_node_put() for
> the from argument.  The callers should be fixed.

I would argue that if everybody makes the same mistake then our
interface is wrong. In that case I wrote it so I think I can plead
guilty to the mistake ;-)

In hindsight, but I don't have the stammina to do a tree-wide change, I
think we should have differenciated:

of_find_xxx which does *not* drop the reference

of_find_next_xxx which does

Cheers,
Ben.

Re: [RFC/PATCH] of: of_find_node_by_name - stop dropping reference to 'from' node

[Benjamin Herrenschmidt]

On Thu, 2016-04-21 at 15:35 -0700, Frank Rowand wrote:
> No.  It is correct for of_find_by_name() to call of_node_put() for
> the from argument.  The callers should be fixed.

I would argue that if everybody makes the same mistake then our
interface is wrong. In that case I wrote it so I think I can plead
guilty to the mistake ;-)

In hindsight, but I don't have the stammina to do a tree-wide change, I
think we should have differenciated:

of_find_xxx which does *not* drop the reference

of_find_next_xxx which does

Cheers,
Ben.

Re: [PATCH] ARM64: dts: rockchip: add core dtsi file for RK3399 SoCs

[Doug Anderson]

Hi,

I didn't look as deeply as Heiko, but a few comments...


On Thu, Apr 21, 2016 at 3:02 PM, Heiko Stübner  wrote:
> Hi Jay,
>
> Am Donnerstag, 21. April 2016, 11:58:12 schrieb Jianqun Xu:
>> This patch adds rk3399.dtsi for rk3399 found on Rockchip
>> RK3399 SoCs, also add rk3399-evb.dts for Rockchip RK3399
>> Evaluation Board.
>>
>> Patch is tested on RK3399 evb.
>>
>> Signed-off-by: Jianqun Xu 
>
> please split this into
> - patch adding the dtsi
> - patch adding the evb dts
> - patch adding the new  board to bindings/arm/rockchip.txt
>
> more inline below

Also don't forget to remove the controversial pmu bits for now (as
discussed earlier) so this can land while all those kinks are being
worked out.

>> + sdhci: sdhci@fe33 {
>> + compatible = "arasan,sdhci-5.1";
>
> not 100% sure, but we might want a
> compatible = "rockchip,rk3399-sdhci-5.1", "arasan,sdhci-5.1";
>
> allowing us to get more specific, if implementation oddities surface later.

I agree with Heiko.  This sounds very sane to me, too, and matches
previous discussions.

>> + reg = <0x0 0xfe33 0x0 0x1>;
>> + interrupts = ;
>> + clocks = < SCLK_EMMC>, < ACLK_EMMC>;
>> + clock-names = "clk_xin", "clk_ahb";
>> + phys = <_phy>;
>> + phy-names = "phy_arasan";
>> + status = "disabled";
>> + };
>> +
>> + usb2phy: usb2phy {
>> + compatible = "rockchip,rk3399-usb-phy";
>
> this doesn't look like it got submitted yet.
>
> Also, the newer socs (rk3399. rk3036, rk3228) seem to use a different usbphy
> block than rk3288 and before (with a big bunch of new phy-related register
> blocks I haven't looked at yet) - so this should probably get a new driver as
> well and not be crammed into the current phy driver, which is for the older
> picophy (or what it was called).
>
>
>> + rockchip,grf = <>;
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + usb2phy0: usb2-phy0 {
>> + #phy-cells = <0>;
>> + #clock-cells = <0>;
>> + reg = <0xe458>;
>> + };
>
> When we're doing a new driver, could we please get rid of these subnodes and
> instead access phys via something like
>
> phys = < 0>;

>From what I recall during the submission of the previous PHY Kishon
preferred the subnodes.  I think I made a fool of myself in the last
discussion about this because I reported bugs in my downstream kernel
that didn't exist upstream, but if you want to read it you can see
here:

https://patchwork.kernel.org/patch/5474871/

I believe patch v6 used IDs like Heiko is suggesting and it turned to
subnodes in v7 based on Kishon's request.  Since PHY code and bindings
are Kishon's call, I have a feeling his opinion will trump here.

>> +
>> + usb2phy1: usb2-phy1 {
>> + #phy-cells = <0>;
>> + #clock-cells = <0>;
>> + reg = <0xe468>;
>> + };
>> + };
>> +
>> + usb_host0_echi: usb@fe38 {
>
> not "echi" please :-)

Just because it took me an extra reading to understand, he means turn
"echi" to "ehci".


>> + compatible = "generic-ehci";
>> + reg = <0x0 0xfe38 0x0 0x2>;
>> + interrupts = ;
>> + clocks = < HCLK_HOST0>, < HCLK_HOST0_ARB>;
>> + clock-names = "hclk_host0", "hclk_host0_arb";
>> + phys = <>;
>> + phy-names = "usb2_phy0";
>> + status = "disabled";
>> + };
>
> [...]
>
>> + usbdrd3_0: usb@fe80 {
>> + compatible = "rockchip,dwc3";
>
> is this in some tree already?

I'm really surprised that there's not some generic fallback for
"dwc3-of-simple.c".  I would have expected:
  "rockchip,rk3399-dwc3", "synopsis,dwc3";

...but that doesn't appear to be in the bindings.  Weird.

>> + i2c1: i2c@ff11 {
>> + compatible = "rockchip,rk3399-i2c";
>
> David respun the rk3399 i2c-support on tuesday, so this and the others below
> are waiting on Wolfram to take a look.

I think it can work with the rk3288-i2c as a fallback, at least for
low speed stuff, right?  Should this be:

compatible = "rockchip,rk3399-i2c", "rockchip,rk3288-i2c"

Looks like that was done for rk3368.


-Doug

Re: [PATCH] ARM64: dts: rockchip: add core dtsi file for RK3399 SoCs

[Doug Anderson]

Hi,

I didn't look as deeply as Heiko, but a few comments...


On Thu, Apr 21, 2016 at 3:02 PM, Heiko Stübner  wrote:
> Hi Jay,
>
> Am Donnerstag, 21. April 2016, 11:58:12 schrieb Jianqun Xu:
>> This patch adds rk3399.dtsi for rk3399 found on Rockchip
>> RK3399 SoCs, also add rk3399-evb.dts for Rockchip RK3399
>> Evaluation Board.
>>
>> Patch is tested on RK3399 evb.
>>
>> Signed-off-by: Jianqun Xu 
>
> please split this into
> - patch adding the dtsi
> - patch adding the evb dts
> - patch adding the new  board to bindings/arm/rockchip.txt
>
> more inline below

Also don't forget to remove the controversial pmu bits for now (as
discussed earlier) so this can land while all those kinks are being
worked out.

>> + sdhci: sdhci@fe33 {
>> + compatible = "arasan,sdhci-5.1";
>
> not 100% sure, but we might want a
> compatible = "rockchip,rk3399-sdhci-5.1", "arasan,sdhci-5.1";
>
> allowing us to get more specific, if implementation oddities surface later.

I agree with Heiko.  This sounds very sane to me, too, and matches
previous discussions.

>> + reg = <0x0 0xfe33 0x0 0x1>;
>> + interrupts = ;
>> + clocks = < SCLK_EMMC>, < ACLK_EMMC>;
>> + clock-names = "clk_xin", "clk_ahb";
>> + phys = <_phy>;
>> + phy-names = "phy_arasan";
>> + status = "disabled";
>> + };
>> +
>> + usb2phy: usb2phy {
>> + compatible = "rockchip,rk3399-usb-phy";
>
> this doesn't look like it got submitted yet.
>
> Also, the newer socs (rk3399. rk3036, rk3228) seem to use a different usbphy
> block than rk3288 and before (with a big bunch of new phy-related register
> blocks I haven't looked at yet) - so this should probably get a new driver as
> well and not be crammed into the current phy driver, which is for the older
> picophy (or what it was called).
>
>
>> + rockchip,grf = <>;
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + usb2phy0: usb2-phy0 {
>> + #phy-cells = <0>;
>> + #clock-cells = <0>;
>> + reg = <0xe458>;
>> + };
>
> When we're doing a new driver, could we please get rid of these subnodes and
> instead access phys via something like
>
> phys = < 0>;

>From what I recall during the submission of the previous PHY Kishon
preferred the subnodes.  I think I made a fool of myself in the last
discussion about this because I reported bugs in my downstream kernel
that didn't exist upstream, but if you want to read it you can see
here:

https://patchwork.kernel.org/patch/5474871/

I believe patch v6 used IDs like Heiko is suggesting and it turned to
subnodes in v7 based on Kishon's request.  Since PHY code and bindings
are Kishon's call, I have a feeling his opinion will trump here.

>> +
>> + usb2phy1: usb2-phy1 {
>> + #phy-cells = <0>;
>> + #clock-cells = <0>;
>> + reg = <0xe468>;
>> + };
>> + };
>> +
>> + usb_host0_echi: usb@fe38 {
>
> not "echi" please :-)

Just because it took me an extra reading to understand, he means turn
"echi" to "ehci".


>> + compatible = "generic-ehci";
>> + reg = <0x0 0xfe38 0x0 0x2>;
>> + interrupts = ;
>> + clocks = < HCLK_HOST0>, < HCLK_HOST0_ARB>;
>> + clock-names = "hclk_host0", "hclk_host0_arb";
>> + phys = <>;
>> + phy-names = "usb2_phy0";
>> + status = "disabled";
>> + };
>
> [...]
>
>> + usbdrd3_0: usb@fe80 {
>> + compatible = "rockchip,dwc3";
>
> is this in some tree already?

I'm really surprised that there's not some generic fallback for
"dwc3-of-simple.c".  I would have expected:
  "rockchip,rk3399-dwc3", "synopsis,dwc3";

...but that doesn't appear to be in the bindings.  Weird.

>> + i2c1: i2c@ff11 {
>> + compatible = "rockchip,rk3399-i2c";
>
> David respun the rk3399 i2c-support on tuesday, so this and the others below
> are waiting on Wolfram to take a look.

I think it can work with the rk3288-i2c as a fallback, at least for
low speed stuff, right?  Should this be:

compatible = "rockchip,rk3399-i2c", "rockchip,rk3288-i2c"

Looks like that was done for rk3368.


-Doug

Re: [PATCH] Input: synaptics-rmi4: Support regulator supplies

[Bjorn Andersson]

On Thu 31 Mar 18:47 PDT 2016, Andrew Duggan wrote:

> On 03/31/2016 12:14 PM, Bjorn Andersson wrote:
> >On Thu 31 Mar 11:19 PDT 2016, Dmitry Torokhov wrote:
> >
> >>Hi Bjorn,
> >>
> >>On Wed, Mar 30, 2016 at 09:57:29AM -0700, Bjorn Andersson wrote:
> >>>From: Bjorn Andersson 
> >>>
> >>>Support the two supplies - vdd and vio - to make it possible to control
> >>>power to the Synaptics chip.
> >>>
> >>>Signed-off-by: Bjorn Andersson 
> >>>Signed-off-by: Bjorn Andersson 
> >>>---
> >>>  .../devicetree/bindings/input/rmi4/rmi_i2c.txt |  7 
> >>>  drivers/input/rmi4/rmi_i2c.c   | 45 
> >>> ++
> >>Would not we need pretty much the same changes for SPI devices? Can this
> >>be done in core?
> >>
> >Yes, I believe it needs the exact same steps.
> >
> >I did a initial quick hack on v1 of the patchset and back then it was
> >possible, when I rebased it a few weeks back I kept ending up in getting
> >interrupts with the power off.
> >
> >Looking at the code this is likely because in the resume paths the IRQ
> >is enabled before we jump to rmi_driver_resume(), so putting this in the
> >core I ended up calling rmi_process_interrupt_requests() before powering
> >up the chip.
> 
> Actually, I don't think the irq needs to be enabled before calling
> rmi_driver_resume(). Typically, the functions are just reading and writing
> to registers and do not need to handle interrupts. We could probably call to
> rmi_driver_resume() before enabling the irq. I can double check that there
> are not any exceptions to this.
> 

I finally got back to giving this a spin.

The problem is that we register the transport device with the driver,
which triggers the rmi_driver probe() which resolves the resources. We
then continue on and call rmi_i2c_init_irq() which will (implicitly)
enable the irq. So if the rmi_driver probe() does not finish in a serial
fashion we will enable interrupts before we have fully initialized the
core.

I don't know if this causes other issues, but with the required delay
after enabling the regulators we always get an interrupt before the
rmi_driver probe() function is finished.

> I have also considered adding a power callback to the core so that the
> transport drivers can set the power independently of suspend and resume. One
> example would be to shut off power to a touchpad if a mouse is connected. If
> we do need to have the irq enabled before calling rmi_driver_resume() we
> could still move regulator support to the core and call the power callback
> from the transport drivers.
> 

I see no (sane) way of waiting for the rmi_driver to finish probeing;
there could be cases where it's powered by a regulator (or reset gpio)
that is not yet probed. EPROBE_DEFER will handle this, but we can't wait
for it in the transport driver.


I therefor think these physical resources should be handled in the
context of the transport layer, to make sure we don't have temporal
dependencies to the other layers.

Or we should not have the rmi_driver as a separate device driver at all
- it could be a "library" that runs in the context of the transport
device.

Regards,
Bjorn

Re: [PATCH] Input: synaptics-rmi4: Support regulator supplies

[Bjorn Andersson]

On Thu 31 Mar 18:47 PDT 2016, Andrew Duggan wrote:

> On 03/31/2016 12:14 PM, Bjorn Andersson wrote:
> >On Thu 31 Mar 11:19 PDT 2016, Dmitry Torokhov wrote:
> >
> >>Hi Bjorn,
> >>
> >>On Wed, Mar 30, 2016 at 09:57:29AM -0700, Bjorn Andersson wrote:
> >>>From: Bjorn Andersson 
> >>>
> >>>Support the two supplies - vdd and vio - to make it possible to control
> >>>power to the Synaptics chip.
> >>>
> >>>Signed-off-by: Bjorn Andersson 
> >>>Signed-off-by: Bjorn Andersson 
> >>>---
> >>>  .../devicetree/bindings/input/rmi4/rmi_i2c.txt |  7 
> >>>  drivers/input/rmi4/rmi_i2c.c   | 45 
> >>> ++
> >>Would not we need pretty much the same changes for SPI devices? Can this
> >>be done in core?
> >>
> >Yes, I believe it needs the exact same steps.
> >
> >I did a initial quick hack on v1 of the patchset and back then it was
> >possible, when I rebased it a few weeks back I kept ending up in getting
> >interrupts with the power off.
> >
> >Looking at the code this is likely because in the resume paths the IRQ
> >is enabled before we jump to rmi_driver_resume(), so putting this in the
> >core I ended up calling rmi_process_interrupt_requests() before powering
> >up the chip.
> 
> Actually, I don't think the irq needs to be enabled before calling
> rmi_driver_resume(). Typically, the functions are just reading and writing
> to registers and do not need to handle interrupts. We could probably call to
> rmi_driver_resume() before enabling the irq. I can double check that there
> are not any exceptions to this.
> 

I finally got back to giving this a spin.

The problem is that we register the transport device with the driver,
which triggers the rmi_driver probe() which resolves the resources. We
then continue on and call rmi_i2c_init_irq() which will (implicitly)
enable the irq. So if the rmi_driver probe() does not finish in a serial
fashion we will enable interrupts before we have fully initialized the
core.

I don't know if this causes other issues, but with the required delay
after enabling the regulators we always get an interrupt before the
rmi_driver probe() function is finished.

> I have also considered adding a power callback to the core so that the
> transport drivers can set the power independently of suspend and resume. One
> example would be to shut off power to a touchpad if a mouse is connected. If
> we do need to have the irq enabled before calling rmi_driver_resume() we
> could still move regulator support to the core and call the power callback
> from the transport drivers.
> 

I see no (sane) way of waiting for the rmi_driver to finish probeing;
there could be cases where it's powered by a regulator (or reset gpio)
that is not yet probed. EPROBE_DEFER will handle this, but we can't wait
for it in the transport driver.


I therefor think these physical resources should be handled in the
context of the transport layer, to make sure we don't have temporal
dependencies to the other layers.

Or we should not have the rmi_driver as a separate device driver at all
- it could be a "library" that runs in the context of the transport
device.

Regards,
Bjorn

Re: [RFC/PATCH] of: of_find_node_by_name - stop dropping reference to 'from' node

[Frank Rowand]

On 4/19/2016 10:05 AM, Dmitry Torokhov wrote:
> Majority of the callers of of_find_node_by_name() do not expect that it
> will drop reference to the 'from' node if it was passed in, causing
> potential refcount underflows, etc, so let's stop doing this.
> 
> Most of the callers that were handling dropping of reference done by
> of_find_node_by_name() actually wanted for_each_node_by_name() instead.
> 
> Signed-off-by: Dmitry Torokhov 
> ---
> 
> If this is acceptable I can make changes to other of_find_node_*()
> methods...

No.  It is correct for of_find_by_name() to call of_node_put() for
the from argument.  The callers should be fixed.

-Frank

> 
>  arch/arm/mach-omap2/omap_hwmod_3xxx_data.c |  2 ++
>  arch/powerpc/platforms/83xx/mpc832x_mds.c  |  2 +-
>  arch/powerpc/platforms/83xx/mpc832x_rdb.c  |  2 +-
>  arch/powerpc/platforms/83xx/mpc836x_mds.c  |  2 +-
>  arch/powerpc/platforms/cell/interrupt.c|  3 +--
>  arch/powerpc/platforms/cell/setup.c|  3 +--
>  arch/powerpc/platforms/cell/spider-pic.c   |  3 +--
>  arch/powerpc/platforms/powermac/feature.c  |  2 +-
>  arch/powerpc/platforms/powermac/pic.c  |  2 --
>  drivers/input/misc/twl4030-vibra.c |  8 +---
>  drivers/of/base.c  |  3 +--
>  drivers/pci/hotplug/rpadlpar_core.c|  4 ++--
>  drivers/video/backlight/tps65217_bl.c  |  4 ++--
>  include/linux/of.h | 12 +---
>  14 files changed, 24 insertions(+), 28 deletions(-)

< snip >

> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index b299de2..45fc458 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -826,7 +826,7 @@ EXPORT_SYMBOL(of_find_node_opts_by_path);
>   *   @from:  The node to start searching from or NULL, the node
>   *   you pass will not be searched, only the next one
>   *   will; typically, you pass what the previous call
> - *   returned. of_node_put() will be called on it
> + *   returned.
>   *   @name:  The name string to match against
>   *
>   *   Returns a node pointer with refcount incremented, use
> @@ -843,7 +843,6 @@ struct device_node *of_find_node_by_name(struct 
> device_node *from,
>   if (np->name && (of_node_cmp(np->name, name) == 0)
>   && of_node_get(np))
>   break;
> - of_node_put(from);
>   raw_spin_unlock_irqrestore(_lock, flags);
>   return np;
>  }

< snip >

Re: [RFC/PATCH] of: of_find_node_by_name - stop dropping reference to 'from' node

[Frank Rowand]

On 4/19/2016 10:05 AM, Dmitry Torokhov wrote:
> Majority of the callers of of_find_node_by_name() do not expect that it
> will drop reference to the 'from' node if it was passed in, causing
> potential refcount underflows, etc, so let's stop doing this.
> 
> Most of the callers that were handling dropping of reference done by
> of_find_node_by_name() actually wanted for_each_node_by_name() instead.
> 
> Signed-off-by: Dmitry Torokhov 
> ---
> 
> If this is acceptable I can make changes to other of_find_node_*()
> methods...

No.  It is correct for of_find_by_name() to call of_node_put() for
the from argument.  The callers should be fixed.

-Frank

> 
>  arch/arm/mach-omap2/omap_hwmod_3xxx_data.c |  2 ++
>  arch/powerpc/platforms/83xx/mpc832x_mds.c  |  2 +-
>  arch/powerpc/platforms/83xx/mpc832x_rdb.c  |  2 +-
>  arch/powerpc/platforms/83xx/mpc836x_mds.c  |  2 +-
>  arch/powerpc/platforms/cell/interrupt.c|  3 +--
>  arch/powerpc/platforms/cell/setup.c|  3 +--
>  arch/powerpc/platforms/cell/spider-pic.c   |  3 +--
>  arch/powerpc/platforms/powermac/feature.c  |  2 +-
>  arch/powerpc/platforms/powermac/pic.c  |  2 --
>  drivers/input/misc/twl4030-vibra.c |  8 +---
>  drivers/of/base.c  |  3 +--
>  drivers/pci/hotplug/rpadlpar_core.c|  4 ++--
>  drivers/video/backlight/tps65217_bl.c  |  4 ++--
>  include/linux/of.h | 12 +---
>  14 files changed, 24 insertions(+), 28 deletions(-)

< snip >

> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index b299de2..45fc458 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -826,7 +826,7 @@ EXPORT_SYMBOL(of_find_node_opts_by_path);
>   *   @from:  The node to start searching from or NULL, the node
>   *   you pass will not be searched, only the next one
>   *   will; typically, you pass what the previous call
> - *   returned. of_node_put() will be called on it
> + *   returned.
>   *   @name:  The name string to match against
>   *
>   *   Returns a node pointer with refcount incremented, use
> @@ -843,7 +843,6 @@ struct device_node *of_find_node_by_name(struct 
> device_node *from,
>   if (np->name && (of_node_cmp(np->name, name) == 0)
>   && of_node_get(np))
>   break;
> - of_node_put(from);
>   raw_spin_unlock_irqrestore(_lock, flags);
>   return np;
>  }

< snip >

Re: [PATCH] ARM64: dts: rockchip: add core dtsi file for rk3399 SoCs

[Heiko Stübner]

Am Donnerstag, 21. April 2016, 14:48:26 schrieb Brian Norris:
> Hi,
> 
> On Wed, Apr 20, 2016 at 11:15:50AM +0800, Jianqun Xu wrote:
> > This patch adds core dtsi file for rk3399 found on Rockchip
> > rk3399 SoCs, tested on rk3399 evb.
> > 
> > Signed-off-by: Jianqun Xu 
> > ---
> > 
> >  arch/arm64/boot/dts/rockchip/rk3399.dtsi | 1744
> >  ++ 1 file changed, 1744 insertions(+)
> >  create mode 100644 arch/arm64/boot/dts/rockchip/rk3399.dtsi
> > 
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> > b/arch/arm64/boot/dts/rockchip/rk3399.dtsi new file mode 100644
> > index 000..fa6fc2c
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> > @@ -0,0 +1,1744 @@
> 
> [...]
> 
> > +   emmc_phy: phy {
> > +   compatible = "rockchip,rk3399-emmc-phy";
> > +   reg-offset = <0xf780>;
> 
> This property is not documented. The current doc says we "require" reg,
> but you're kinda misusing it, I believe. At any rate, the current phy
> driver won't probe without 'reg'.

thanks for spotting this.

Please also note the changed binding [0] that I'm still hoping
will make it into 4.6.



[0] 
https://git.kernel.org/cgit/linux/kernel/git/kishon/linux-phy.git/log/?h=fixes


> > +   #phy-cells = <0>;
> > +   rockchip,grf = <>;
> > +   status = "disabled";
> > +   };
> 
> [...]
> 
> Brian

Re: [PATCH] ARM64: dts: rockchip: add core dtsi file for rk3399 SoCs

[Heiko Stübner]

Am Donnerstag, 21. April 2016, 14:48:26 schrieb Brian Norris:
> Hi,
> 
> On Wed, Apr 20, 2016 at 11:15:50AM +0800, Jianqun Xu wrote:
> > This patch adds core dtsi file for rk3399 found on Rockchip
> > rk3399 SoCs, tested on rk3399 evb.
> > 
> > Signed-off-by: Jianqun Xu 
> > ---
> > 
> >  arch/arm64/boot/dts/rockchip/rk3399.dtsi | 1744
> >  ++ 1 file changed, 1744 insertions(+)
> >  create mode 100644 arch/arm64/boot/dts/rockchip/rk3399.dtsi
> > 
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> > b/arch/arm64/boot/dts/rockchip/rk3399.dtsi new file mode 100644
> > index 000..fa6fc2c
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> > @@ -0,0 +1,1744 @@
> 
> [...]
> 
> > +   emmc_phy: phy {
> > +   compatible = "rockchip,rk3399-emmc-phy";
> > +   reg-offset = <0xf780>;
> 
> This property is not documented. The current doc says we "require" reg,
> but you're kinda misusing it, I believe. At any rate, the current phy
> driver won't probe without 'reg'.

thanks for spotting this.

Please also note the changed binding [0] that I'm still hoping
will make it into 4.6.



[0] 
https://git.kernel.org/cgit/linux/kernel/git/kishon/linux-phy.git/log/?h=fixes


> > +   #phy-cells = <0>;
> > +   rockchip,grf = <>;
> > +   status = "disabled";
> > +   };
> 
> [...]
> 
> Brian

[GIT PULL] RTC fixes for 4.6

[Alexandre Belloni]

Hi Linus,

A few fixes for the RTC subsystem. The documentation fix already missed
4.5 so I think it is worth taking it now.

Note that I'm now using a signing subkey so you may need to refresh my
GPG key.

The following changes since commit f55532a0c0b8bb6148f4e07853b876ef73bc69ca:

  Linux 4.6-rc1 (2016-03-26 16:03:24 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux.git 
tags/rtc-4.6-3

for you to fetch changes up to 38a7a73e8ed0fa9e8cd99bb62463afbdcd23429b:

  rtc: ds1307: Use irq when available for wakeup-source device (2016-04-21 
23:21:00 +0200)


RTC fixes for 4.6

A documentation fix for s3c and two fixes for the ds1307.


Javier Martinez Canillas (1):
  rtc: s3c: Document in binding that only s3c6410 needs a src clk

Nishanth Menon (1):
  rtc: ds1307: Use irq when available for wakeup-source device

Zhuang Yuyao (1):
  rtc: ds1307: ds3231 temperature s16 overflow

 Documentation/devicetree/bindings/rtc/s3c-rtc.txt | 7 ---
 drivers/rtc/rtc-ds1307.c  | 6 +++---
 2 files changed, 7 insertions(+), 6 deletions(-)

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


signature.asc
Description: PGP signature

[GIT PULL] RTC fixes for 4.6

[Alexandre Belloni]

Hi Linus,

A few fixes for the RTC subsystem. The documentation fix already missed
4.5 so I think it is worth taking it now.

Note that I'm now using a signing subkey so you may need to refresh my
GPG key.

The following changes since commit f55532a0c0b8bb6148f4e07853b876ef73bc69ca:

  Linux 4.6-rc1 (2016-03-26 16:03:24 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux.git 
tags/rtc-4.6-3

for you to fetch changes up to 38a7a73e8ed0fa9e8cd99bb62463afbdcd23429b:

  rtc: ds1307: Use irq when available for wakeup-source device (2016-04-21 
23:21:00 +0200)


RTC fixes for 4.6

A documentation fix for s3c and two fixes for the ds1307.


Javier Martinez Canillas (1):
  rtc: s3c: Document in binding that only s3c6410 needs a src clk

Nishanth Menon (1):
  rtc: ds1307: Use irq when available for wakeup-source device

Zhuang Yuyao (1):
  rtc: ds1307: ds3231 temperature s16 overflow

 Documentation/devicetree/bindings/rtc/s3c-rtc.txt | 7 ---
 drivers/rtc/rtc-ds1307.c  | 6 +++---
 2 files changed, 7 insertions(+), 6 deletions(-)

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


signature.asc
Description: PGP signature

Re: [alsa-devel] [PATCH 4/4] ASoC: simple-card: Support for selecting system clocks by ID

[Stephen Boyd]

On 04/18, Mark Brown wrote:
> On Mon, Apr 18, 2016 at 06:50:52PM +0300, Peter Ujfalusi wrote:
> > On 02/17/2016 09:52 PM, Peter Ujfalusi wrote:
> 
> > > On the other hand this ABI is backwards compatible since if it is missing 
> > > it
> > > will default to the configuration we right now have regarding to 
> > > sysclk_dir
> > > and sysclk_id.
> 
> > > I will look at the CCF implementation for McASP first then for aic3x.
> 
> > The first issue with converting the McASP to use CCF internally for clock
> > selection, muxing and rate configuration is that the daVinci platform does 
> > not
> > use CCF at all. Given that the davinci-mcasp driver is used by daVinci, we
> > need to have non CCF way supported in ASoC...
> 
> Well, at least long term we do need daVinci converting to CCF - this is
> going to continue to cause problems, devices not part of the SoC can and
> do contain clocks and are going to end up being supported via the clock
> API.

Does anyone here know what's involved in converting daVinci to
CCF? It doesn't look too far off from what is in the CCF today,
so I'm not sure what's blocking the transition.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [alsa-devel] [PATCH 4/4] ASoC: simple-card: Support for selecting system clocks by ID

[Stephen Boyd]

On 04/18, Mark Brown wrote:
> On Mon, Apr 18, 2016 at 06:50:52PM +0300, Peter Ujfalusi wrote:
> > On 02/17/2016 09:52 PM, Peter Ujfalusi wrote:
> 
> > > On the other hand this ABI is backwards compatible since if it is missing 
> > > it
> > > will default to the configuration we right now have regarding to 
> > > sysclk_dir
> > > and sysclk_id.
> 
> > > I will look at the CCF implementation for McASP first then for aic3x.
> 
> > The first issue with converting the McASP to use CCF internally for clock
> > selection, muxing and rate configuration is that the daVinci platform does 
> > not
> > use CCF at all. Given that the davinci-mcasp driver is used by daVinci, we
> > need to have non CCF way supported in ASoC...
> 
> Well, at least long term we do need daVinci converting to CCF - this is
> going to continue to cause problems, devices not part of the SoC can and
> do contain clocks and are going to end up being supported via the clock
> API.

Does anyone here know what's involved in converting daVinci to
CCF? It doesn't look too far off from what is in the CCF today,
so I'm not sure what's blocking the transition.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v2 09/13] clk: fractional-divider: Add hw based registration APIs

[Stephen Boyd]

Add registration APIs in the clk fractional divider code to
return struct clk_hw pointers instead of struct clk pointers.
This way we hide the struct clk pointer from providers unless
they need to use consumer facing APIs.

Signed-off-by: Stephen Boyd 
---
 drivers/clk/clk-fractional-divider.c | 40 +++-
 include/linux/clk-provider.h |  5 +
 2 files changed, 40 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/clk-fractional-divider.c 
b/drivers/clk/clk-fractional-divider.c
index 1abcd76b4993..aab904618eb6 100644
--- a/drivers/clk/clk-fractional-divider.c
+++ b/drivers/clk/clk-fractional-divider.c
@@ -116,14 +116,15 @@ const struct clk_ops clk_fractional_divider_ops = {
 };
 EXPORT_SYMBOL_GPL(clk_fractional_divider_ops);
 
-struct clk *clk_register_fractional_divider(struct device *dev,
+struct clk_hw *clk_hw_register_fractional_divider(struct device *dev,
const char *name, const char *parent_name, unsigned long flags,
void __iomem *reg, u8 mshift, u8 mwidth, u8 nshift, u8 nwidth,
u8 clk_divider_flags, spinlock_t *lock)
 {
struct clk_fractional_divider *fd;
struct clk_init_data init;
-   struct clk *clk;
+   struct clk_hw *hw;
+   int ret;
 
fd = kzalloc(sizeof(*fd), GFP_KERNEL);
if (!fd)
@@ -146,10 +147,39 @@ struct clk *clk_register_fractional_divider(struct device 
*dev,
fd->lock = lock;
fd->hw.init = 
 
-   clk = clk_register(dev, >hw);
-   if (IS_ERR(clk))
+   hw = >hw;
+   ret = clk_hw_register(dev, hw);
+   if (ret) {
kfree(fd);
+   hw = ERR_PTR(ret);
+   }
+
+   return hw;
+}
+EXPORT_SYMBOL_GPL(clk_hw_register_fractional_divider);
 
-   return clk;
+struct clk *clk_register_fractional_divider(struct device *dev,
+   const char *name, const char *parent_name, unsigned long flags,
+   void __iomem *reg, u8 mshift, u8 mwidth, u8 nshift, u8 nwidth,
+   u8 clk_divider_flags, spinlock_t *lock)
+{
+   struct clk_hw *hw;
+
+   hw = clk_hw_register_fractional_divider(dev, name, parent_name, flags,
+   reg, mshift, mwidth, nshift, nwidth, clk_divider_flags,
+   lock);
+   if (IS_ERR(hw))
+   return ERR_CAST(hw);
+   return hw->clk;
 }
 EXPORT_SYMBOL_GPL(clk_register_fractional_divider);
+
+void clk_hw_unregister_fractional_divider(struct clk_hw *hw)
+{
+   struct clk_fractional_divider *fd;
+
+   fd = to_clk_fd(hw);
+
+   clk_hw_unregister(hw);
+   kfree(fd);
+}
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 79ad1a8a6831..bcbaf6c95d52 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -563,6 +563,11 @@ struct clk *clk_register_fractional_divider(struct device 
*dev,
const char *name, const char *parent_name, unsigned long flags,
void __iomem *reg, u8 mshift, u8 mwidth, u8 nshift, u8 nwidth,
u8 clk_divider_flags, spinlock_t *lock);
+struct clk_hw *clk_hw_register_fractional_divider(struct device *dev,
+   const char *name, const char *parent_name, unsigned long flags,
+   void __iomem *reg, u8 mshift, u8 mwidth, u8 nshift, u8 nwidth,
+   u8 clk_divider_flags, spinlock_t *lock);
+void clk_hw_unregister_fractional_divider(struct clk_hw *hw);
 
 /**
  * struct clk_multiplier - adjustable multiplier clock
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v2 07/13] clk: mux: Add hw based registration APIs

[Stephen Boyd]

Add registration APIs in the clk mux code to return struct clk_hw
pointers instead of struct clk pointers. This way we hide the
struct clk pointer from providers unless they need to use
consumer facing APIs.

Signed-off-by: Stephen Boyd 
---
 drivers/clk/clk-mux.c| 57 +++-
 include/linux/clk-provider.h | 11 +
 2 files changed, 62 insertions(+), 6 deletions(-)

diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c
index 252188fd8bcd..16a3d5717f4e 100644
--- a/drivers/clk/clk-mux.c
+++ b/drivers/clk/clk-mux.c
@@ -113,16 +113,17 @@ const struct clk_ops clk_mux_ro_ops = {
 };
 EXPORT_SYMBOL_GPL(clk_mux_ro_ops);
 
-struct clk *clk_register_mux_table(struct device *dev, const char *name,
+struct clk_hw *clk_hw_register_mux_table(struct device *dev, const char *name,
const char * const *parent_names, u8 num_parents,
unsigned long flags,
void __iomem *reg, u8 shift, u32 mask,
u8 clk_mux_flags, u32 *table, spinlock_t *lock)
 {
struct clk_mux *mux;
-   struct clk *clk;
+   struct clk_hw *hw;
struct clk_init_data init;
u8 width = 0;
+   int ret;
 
if (clk_mux_flags & CLK_MUX_HIWORD_MASK) {
width = fls(mask) - ffs(mask) + 1;
@@ -157,12 +158,31 @@ struct clk *clk_register_mux_table(struct device *dev, 
const char *name,
mux->table = table;
mux->hw.init = 
 
-   clk = clk_register(dev, >hw);
-
-   if (IS_ERR(clk))
+   hw = >hw;
+   ret = clk_hw_register(dev, hw);
+   if (ret) {
kfree(mux);
+   hw = ERR_PTR(ret);
+   }
 
-   return clk;
+   return hw;
+}
+EXPORT_SYMBOL_GPL(clk_hw_register_mux_table);
+
+struct clk *clk_register_mux_table(struct device *dev, const char *name,
+   const char * const *parent_names, u8 num_parents,
+   unsigned long flags,
+   void __iomem *reg, u8 shift, u32 mask,
+   u8 clk_mux_flags, u32 *table, spinlock_t *lock)
+{
+   struct clk_hw *hw;
+
+   hw = clk_hw_register_mux_table(dev, name, parent_names, num_parents,
+  flags, reg, shift, mask, clk_mux_flags,
+  table, lock);
+   if (IS_ERR(hw))
+   return ERR_CAST(hw);
+   return hw->clk;
 }
 EXPORT_SYMBOL_GPL(clk_register_mux_table);
 
@@ -180,6 +200,20 @@ struct clk *clk_register_mux(struct device *dev, const 
char *name,
 }
 EXPORT_SYMBOL_GPL(clk_register_mux);
 
+struct clk_hw *clk_hw_register_mux(struct device *dev, const char *name,
+   const char * const *parent_names, u8 num_parents,
+   unsigned long flags,
+   void __iomem *reg, u8 shift, u8 width,
+   u8 clk_mux_flags, spinlock_t *lock)
+{
+   u32 mask = BIT(width) - 1;
+
+   return clk_hw_register_mux_table(dev, name, parent_names, num_parents,
+ flags, reg, shift, mask, clk_mux_flags,
+ NULL, lock);
+}
+EXPORT_SYMBOL_GPL(clk_hw_register_mux);
+
 void clk_unregister_mux(struct clk *clk)
 {
struct clk_mux *mux;
@@ -195,3 +229,14 @@ void clk_unregister_mux(struct clk *clk)
kfree(mux);
 }
 EXPORT_SYMBOL_GPL(clk_unregister_mux);
+
+void clk_hw_unregister_mux(struct clk_hw *hw)
+{
+   struct clk_mux *mux;
+
+   mux = to_clk_mux(hw);
+
+   clk_hw_unregister(hw);
+   kfree(mux);
+}
+EXPORT_SYMBOL_GPL(clk_hw_unregister_mux);
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index bf12050aadd5..d690d99b9c1c 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -478,14 +478,25 @@ struct clk *clk_register_mux(struct device *dev, const 
char *name,
unsigned long flags,
void __iomem *reg, u8 shift, u8 width,
u8 clk_mux_flags, spinlock_t *lock);
+struct clk_hw *clk_hw_register_mux(struct device *dev, const char *name,
+   const char * const *parent_names, u8 num_parents,
+   unsigned long flags,
+   void __iomem *reg, u8 shift, u8 width,
+   u8 clk_mux_flags, spinlock_t *lock);
 
 struct clk *clk_register_mux_table(struct device *dev, const char *name,
const char * const *parent_names, u8 num_parents,
unsigned long flags,
void __iomem *reg, u8 shift, u32 mask,
u8 clk_mux_flags, u32 *table, spinlock_t *lock);
+struct clk_hw *clk_hw_register_mux_table(struct device *dev, const char *name,
+   const char * const *parent_names, u8 num_parents,
+   unsigned long flags,
+   void __iomem *reg, u8 shift, u32 mask,
+   u8 clk_mux_flags, u32 *table, spinlock_t *lock);
 
 void clk_unregister_mux(struct clk *clk);
+void clk_hw_unregister_mux(struct clk_hw *hw);
 
 void 

[PATCH v2 09/13] clk: fractional-divider: Add hw based registration APIs

[Stephen Boyd]

Add registration APIs in the clk fractional divider code to
return struct clk_hw pointers instead of struct clk pointers.
This way we hide the struct clk pointer from providers unless
they need to use consumer facing APIs.

Signed-off-by: Stephen Boyd 
---
 drivers/clk/clk-fractional-divider.c | 40 +++-
 include/linux/clk-provider.h |  5 +
 2 files changed, 40 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/clk-fractional-divider.c 
b/drivers/clk/clk-fractional-divider.c
index 1abcd76b4993..aab904618eb6 100644
--- a/drivers/clk/clk-fractional-divider.c
+++ b/drivers/clk/clk-fractional-divider.c
@@ -116,14 +116,15 @@ const struct clk_ops clk_fractional_divider_ops = {
 };
 EXPORT_SYMBOL_GPL(clk_fractional_divider_ops);
 
-struct clk *clk_register_fractional_divider(struct device *dev,
+struct clk_hw *clk_hw_register_fractional_divider(struct device *dev,
const char *name, const char *parent_name, unsigned long flags,
void __iomem *reg, u8 mshift, u8 mwidth, u8 nshift, u8 nwidth,
u8 clk_divider_flags, spinlock_t *lock)
 {
struct clk_fractional_divider *fd;
struct clk_init_data init;
-   struct clk *clk;
+   struct clk_hw *hw;
+   int ret;
 
fd = kzalloc(sizeof(*fd), GFP_KERNEL);
if (!fd)
@@ -146,10 +147,39 @@ struct clk *clk_register_fractional_divider(struct device 
*dev,
fd->lock = lock;
fd->hw.init = 
 
-   clk = clk_register(dev, >hw);
-   if (IS_ERR(clk))
+   hw = >hw;
+   ret = clk_hw_register(dev, hw);
+   if (ret) {
kfree(fd);
+   hw = ERR_PTR(ret);
+   }
+
+   return hw;
+}
+EXPORT_SYMBOL_GPL(clk_hw_register_fractional_divider);
 
-   return clk;
+struct clk *clk_register_fractional_divider(struct device *dev,
+   const char *name, const char *parent_name, unsigned long flags,
+   void __iomem *reg, u8 mshift, u8 mwidth, u8 nshift, u8 nwidth,
+   u8 clk_divider_flags, spinlock_t *lock)
+{
+   struct clk_hw *hw;
+
+   hw = clk_hw_register_fractional_divider(dev, name, parent_name, flags,
+   reg, mshift, mwidth, nshift, nwidth, clk_divider_flags,
+   lock);
+   if (IS_ERR(hw))
+   return ERR_CAST(hw);
+   return hw->clk;
 }
 EXPORT_SYMBOL_GPL(clk_register_fractional_divider);
+
+void clk_hw_unregister_fractional_divider(struct clk_hw *hw)
+{
+   struct clk_fractional_divider *fd;
+
+   fd = to_clk_fd(hw);
+
+   clk_hw_unregister(hw);
+   kfree(fd);
+}
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 79ad1a8a6831..bcbaf6c95d52 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -563,6 +563,11 @@ struct clk *clk_register_fractional_divider(struct device 
*dev,
const char *name, const char *parent_name, unsigned long flags,
void __iomem *reg, u8 mshift, u8 mwidth, u8 nshift, u8 nwidth,
u8 clk_divider_flags, spinlock_t *lock);
+struct clk_hw *clk_hw_register_fractional_divider(struct device *dev,
+   const char *name, const char *parent_name, unsigned long flags,
+   void __iomem *reg, u8 mshift, u8 mwidth, u8 nshift, u8 nwidth,
+   u8 clk_divider_flags, spinlock_t *lock);
+void clk_hw_unregister_fractional_divider(struct clk_hw *hw);
 
 /**
  * struct clk_multiplier - adjustable multiplier clock
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v2 07/13] clk: mux: Add hw based registration APIs

[Stephen Boyd]

Add registration APIs in the clk mux code to return struct clk_hw
pointers instead of struct clk pointers. This way we hide the
struct clk pointer from providers unless they need to use
consumer facing APIs.

Signed-off-by: Stephen Boyd 
---
 drivers/clk/clk-mux.c| 57 +++-
 include/linux/clk-provider.h | 11 +
 2 files changed, 62 insertions(+), 6 deletions(-)

diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c
index 252188fd8bcd..16a3d5717f4e 100644
--- a/drivers/clk/clk-mux.c
+++ b/drivers/clk/clk-mux.c
@@ -113,16 +113,17 @@ const struct clk_ops clk_mux_ro_ops = {
 };
 EXPORT_SYMBOL_GPL(clk_mux_ro_ops);
 
-struct clk *clk_register_mux_table(struct device *dev, const char *name,
+struct clk_hw *clk_hw_register_mux_table(struct device *dev, const char *name,
const char * const *parent_names, u8 num_parents,
unsigned long flags,
void __iomem *reg, u8 shift, u32 mask,
u8 clk_mux_flags, u32 *table, spinlock_t *lock)
 {
struct clk_mux *mux;
-   struct clk *clk;
+   struct clk_hw *hw;
struct clk_init_data init;
u8 width = 0;
+   int ret;
 
if (clk_mux_flags & CLK_MUX_HIWORD_MASK) {
width = fls(mask) - ffs(mask) + 1;
@@ -157,12 +158,31 @@ struct clk *clk_register_mux_table(struct device *dev, 
const char *name,
mux->table = table;
mux->hw.init = 
 
-   clk = clk_register(dev, >hw);
-
-   if (IS_ERR(clk))
+   hw = >hw;
+   ret = clk_hw_register(dev, hw);
+   if (ret) {
kfree(mux);
+   hw = ERR_PTR(ret);
+   }
 
-   return clk;
+   return hw;
+}
+EXPORT_SYMBOL_GPL(clk_hw_register_mux_table);
+
+struct clk *clk_register_mux_table(struct device *dev, const char *name,
+   const char * const *parent_names, u8 num_parents,
+   unsigned long flags,
+   void __iomem *reg, u8 shift, u32 mask,
+   u8 clk_mux_flags, u32 *table, spinlock_t *lock)
+{
+   struct clk_hw *hw;
+
+   hw = clk_hw_register_mux_table(dev, name, parent_names, num_parents,
+  flags, reg, shift, mask, clk_mux_flags,
+  table, lock);
+   if (IS_ERR(hw))
+   return ERR_CAST(hw);
+   return hw->clk;
 }
 EXPORT_SYMBOL_GPL(clk_register_mux_table);
 
@@ -180,6 +200,20 @@ struct clk *clk_register_mux(struct device *dev, const 
char *name,
 }
 EXPORT_SYMBOL_GPL(clk_register_mux);
 
+struct clk_hw *clk_hw_register_mux(struct device *dev, const char *name,
+   const char * const *parent_names, u8 num_parents,
+   unsigned long flags,
+   void __iomem *reg, u8 shift, u8 width,
+   u8 clk_mux_flags, spinlock_t *lock)
+{
+   u32 mask = BIT(width) - 1;
+
+   return clk_hw_register_mux_table(dev, name, parent_names, num_parents,
+ flags, reg, shift, mask, clk_mux_flags,
+ NULL, lock);
+}
+EXPORT_SYMBOL_GPL(clk_hw_register_mux);
+
 void clk_unregister_mux(struct clk *clk)
 {
struct clk_mux *mux;
@@ -195,3 +229,14 @@ void clk_unregister_mux(struct clk *clk)
kfree(mux);
 }
 EXPORT_SYMBOL_GPL(clk_unregister_mux);
+
+void clk_hw_unregister_mux(struct clk_hw *hw)
+{
+   struct clk_mux *mux;
+
+   mux = to_clk_mux(hw);
+
+   clk_hw_unregister(hw);
+   kfree(mux);
+}
+EXPORT_SYMBOL_GPL(clk_hw_unregister_mux);
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index bf12050aadd5..d690d99b9c1c 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -478,14 +478,25 @@ struct clk *clk_register_mux(struct device *dev, const 
char *name,
unsigned long flags,
void __iomem *reg, u8 shift, u8 width,
u8 clk_mux_flags, spinlock_t *lock);
+struct clk_hw *clk_hw_register_mux(struct device *dev, const char *name,
+   const char * const *parent_names, u8 num_parents,
+   unsigned long flags,
+   void __iomem *reg, u8 shift, u8 width,
+   u8 clk_mux_flags, spinlock_t *lock);
 
 struct clk *clk_register_mux_table(struct device *dev, const char *name,
const char * const *parent_names, u8 num_parents,
unsigned long flags,
void __iomem *reg, u8 shift, u32 mask,
u8 clk_mux_flags, u32 *table, spinlock_t *lock);
+struct clk_hw *clk_hw_register_mux_table(struct device *dev, const char *name,
+   const char * const *parent_names, u8 num_parents,
+   unsigned long flags,
+   void __iomem *reg, u8 shift, u32 mask,
+   u8 clk_mux_flags, u32 *table, spinlock_t *lock);
 
 void clk_unregister_mux(struct clk *clk);
+void clk_hw_unregister_mux(struct clk_hw *hw);
 
 void 

[PATCH v2 05/13] clk: divider: Add hw based registration APIs

[Stephen Boyd]

Add registration APIs in the clk divider code to return struct
clk_hw pointers instead of struct clk pointers. This way we hide
the struct clk pointer from providers unless they need to use
consumer facing APIs.

Signed-off-by: Stephen Boyd 
---
 drivers/clk/clk-divider.c| 91 
 include/linux/clk-provider.h | 10 +
 2 files changed, 93 insertions(+), 8 deletions(-)

diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
index 00e035b51c69..a0f55bc1ad3d 100644
--- a/drivers/clk/clk-divider.c
+++ b/drivers/clk/clk-divider.c
@@ -426,15 +426,16 @@ const struct clk_ops clk_divider_ro_ops = {
 };
 EXPORT_SYMBOL_GPL(clk_divider_ro_ops);
 
-static struct clk *_register_divider(struct device *dev, const char *name,
+static struct clk_hw *_register_divider(struct device *dev, const char *name,
const char *parent_name, unsigned long flags,
void __iomem *reg, u8 shift, u8 width,
u8 clk_divider_flags, const struct clk_div_table *table,
spinlock_t *lock)
 {
struct clk_divider *div;
-   struct clk *clk;
+   struct clk_hw *hw;
struct clk_init_data init;
+   int ret;
 
if (clk_divider_flags & CLK_DIVIDER_HIWORD_MASK) {
if (width + shift > 16) {
@@ -467,12 +468,14 @@ static struct clk *_register_divider(struct device *dev, 
const char *name,
div->table = table;
 
/* register the clock */
-   clk = clk_register(dev, >hw);
-
-   if (IS_ERR(clk))
+   hw = >hw;
+   ret = clk_hw_register(dev, hw);
+   if (ret) {
kfree(div);
+   hw = ERR_PTR(ret);
+   }
 
-   return clk;
+   return hw;
 }
 
 /**
@@ -492,12 +495,39 @@ struct clk *clk_register_divider(struct device *dev, 
const char *name,
void __iomem *reg, u8 shift, u8 width,
u8 clk_divider_flags, spinlock_t *lock)
 {
-   return _register_divider(dev, name, parent_name, flags, reg, shift,
+   struct clk_hw *hw;
+
+   hw =  _register_divider(dev, name, parent_name, flags, reg, shift,
width, clk_divider_flags, NULL, lock);
+   if (IS_ERR(hw))
+   return ERR_CAST(hw);
+   return hw->clk;
 }
 EXPORT_SYMBOL_GPL(clk_register_divider);
 
 /**
+ * clk_hw_register_divider - register a divider clock with the clock framework
+ * @dev: device registering this clock
+ * @name: name of this clock
+ * @parent_name: name of clock's parent
+ * @flags: framework-specific flags
+ * @reg: register address to adjust divider
+ * @shift: number of bits to shift the bitfield
+ * @width: width of the bitfield
+ * @clk_divider_flags: divider-specific flags for this clock
+ * @lock: shared register lock for this clock
+ */
+struct clk_hw *clk_hw_register_divider(struct device *dev, const char *name,
+   const char *parent_name, unsigned long flags,
+   void __iomem *reg, u8 shift, u8 width,
+   u8 clk_divider_flags, spinlock_t *lock)
+{
+   return _register_divider(dev, name, parent_name, flags, reg, shift,
+   width, clk_divider_flags, NULL, lock);
+}
+EXPORT_SYMBOL_GPL(clk_hw_register_divider);
+
+/**
  * clk_register_divider_table - register a table based divider clock with
  * the clock framework
  * @dev: device registering this clock
@@ -517,11 +547,41 @@ struct clk *clk_register_divider_table(struct device 
*dev, const char *name,
u8 clk_divider_flags, const struct clk_div_table *table,
spinlock_t *lock)
 {
-   return _register_divider(dev, name, parent_name, flags, reg, shift,
+   struct clk_hw *hw;
+
+   hw =  _register_divider(dev, name, parent_name, flags, reg, shift,
width, clk_divider_flags, table, lock);
+   if (IS_ERR(hw))
+   return ERR_CAST(hw);
+   return hw->clk;
 }
 EXPORT_SYMBOL_GPL(clk_register_divider_table);
 
+/**
+ * clk_hw_register_divider_table - register a table based divider clock with
+ * the clock framework
+ * @dev: device registering this clock
+ * @name: name of this clock
+ * @parent_name: name of clock's parent
+ * @flags: framework-specific flags
+ * @reg: register address to adjust divider
+ * @shift: number of bits to shift the bitfield
+ * @width: width of the bitfield
+ * @clk_divider_flags: divider-specific flags for this clock
+ * @table: array of divider/value pairs ending with a div set to 0
+ * @lock: shared register lock for this clock
+ */
+struct clk_hw *clk_hw_register_divider_table(struct device *dev,
+   const char *name, const char *parent_name, unsigned long flags,
+   void __iomem *reg, u8 shift, u8 width,
+   u8 clk_divider_flags, const struct clk_div_table *table,
+   spinlock_t *lock)
+{
+   return _register_divider(dev, name, parent_name, flags, reg, shift,
+   width, 

[PATCH v2 05/13] clk: divider: Add hw based registration APIs

[Stephen Boyd]

Add registration APIs in the clk divider code to return struct
clk_hw pointers instead of struct clk pointers. This way we hide
the struct clk pointer from providers unless they need to use
consumer facing APIs.

Signed-off-by: Stephen Boyd 
---
 drivers/clk/clk-divider.c| 91 
 include/linux/clk-provider.h | 10 +
 2 files changed, 93 insertions(+), 8 deletions(-)

diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
index 00e035b51c69..a0f55bc1ad3d 100644
--- a/drivers/clk/clk-divider.c
+++ b/drivers/clk/clk-divider.c
@@ -426,15 +426,16 @@ const struct clk_ops clk_divider_ro_ops = {
 };
 EXPORT_SYMBOL_GPL(clk_divider_ro_ops);
 
-static struct clk *_register_divider(struct device *dev, const char *name,
+static struct clk_hw *_register_divider(struct device *dev, const char *name,
const char *parent_name, unsigned long flags,
void __iomem *reg, u8 shift, u8 width,
u8 clk_divider_flags, const struct clk_div_table *table,
spinlock_t *lock)
 {
struct clk_divider *div;
-   struct clk *clk;
+   struct clk_hw *hw;
struct clk_init_data init;
+   int ret;
 
if (clk_divider_flags & CLK_DIVIDER_HIWORD_MASK) {
if (width + shift > 16) {
@@ -467,12 +468,14 @@ static struct clk *_register_divider(struct device *dev, 
const char *name,
div->table = table;
 
/* register the clock */
-   clk = clk_register(dev, >hw);
-
-   if (IS_ERR(clk))
+   hw = >hw;
+   ret = clk_hw_register(dev, hw);
+   if (ret) {
kfree(div);
+   hw = ERR_PTR(ret);
+   }
 
-   return clk;
+   return hw;
 }
 
 /**
@@ -492,12 +495,39 @@ struct clk *clk_register_divider(struct device *dev, 
const char *name,
void __iomem *reg, u8 shift, u8 width,
u8 clk_divider_flags, spinlock_t *lock)
 {
-   return _register_divider(dev, name, parent_name, flags, reg, shift,
+   struct clk_hw *hw;
+
+   hw =  _register_divider(dev, name, parent_name, flags, reg, shift,
width, clk_divider_flags, NULL, lock);
+   if (IS_ERR(hw))
+   return ERR_CAST(hw);
+   return hw->clk;
 }
 EXPORT_SYMBOL_GPL(clk_register_divider);
 
 /**
+ * clk_hw_register_divider - register a divider clock with the clock framework
+ * @dev: device registering this clock
+ * @name: name of this clock
+ * @parent_name: name of clock's parent
+ * @flags: framework-specific flags
+ * @reg: register address to adjust divider
+ * @shift: number of bits to shift the bitfield
+ * @width: width of the bitfield
+ * @clk_divider_flags: divider-specific flags for this clock
+ * @lock: shared register lock for this clock
+ */
+struct clk_hw *clk_hw_register_divider(struct device *dev, const char *name,
+   const char *parent_name, unsigned long flags,
+   void __iomem *reg, u8 shift, u8 width,
+   u8 clk_divider_flags, spinlock_t *lock)
+{
+   return _register_divider(dev, name, parent_name, flags, reg, shift,
+   width, clk_divider_flags, NULL, lock);
+}
+EXPORT_SYMBOL_GPL(clk_hw_register_divider);
+
+/**
  * clk_register_divider_table - register a table based divider clock with
  * the clock framework
  * @dev: device registering this clock
@@ -517,11 +547,41 @@ struct clk *clk_register_divider_table(struct device 
*dev, const char *name,
u8 clk_divider_flags, const struct clk_div_table *table,
spinlock_t *lock)
 {
-   return _register_divider(dev, name, parent_name, flags, reg, shift,
+   struct clk_hw *hw;
+
+   hw =  _register_divider(dev, name, parent_name, flags, reg, shift,
width, clk_divider_flags, table, lock);
+   if (IS_ERR(hw))
+   return ERR_CAST(hw);
+   return hw->clk;
 }
 EXPORT_SYMBOL_GPL(clk_register_divider_table);
 
+/**
+ * clk_hw_register_divider_table - register a table based divider clock with
+ * the clock framework
+ * @dev: device registering this clock
+ * @name: name of this clock
+ * @parent_name: name of clock's parent
+ * @flags: framework-specific flags
+ * @reg: register address to adjust divider
+ * @shift: number of bits to shift the bitfield
+ * @width: width of the bitfield
+ * @clk_divider_flags: divider-specific flags for this clock
+ * @table: array of divider/value pairs ending with a div set to 0
+ * @lock: shared register lock for this clock
+ */
+struct clk_hw *clk_hw_register_divider_table(struct device *dev,
+   const char *name, const char *parent_name, unsigned long flags,
+   void __iomem *reg, u8 shift, u8 width,
+   u8 clk_divider_flags, const struct clk_div_table *table,
+   spinlock_t *lock)
+{
+   return _register_divider(dev, name, parent_name, flags, reg, shift,
+   width, clk_divider_flags, table, 

[PATCH v2 01/13] clkdev: Remove clk_register_clkdevs()

[Stephen Boyd]

Now that we've converted the only caller over to another clkdev
API, remove this one.

Reviewed-by: Andy Shevchenko 
Cc: Russell King 
Signed-off-by: Stephen Boyd 
---
 drivers/clk/clkdev.c   | 27 ---
 include/linux/clkdev.h |  1 -
 2 files changed, 28 deletions(-)

diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c
index eb20b941154b..ae8e40a82d34 100644
--- a/drivers/clk/clkdev.c
+++ b/drivers/clk/clkdev.c
@@ -402,30 +402,3 @@ int clk_register_clkdev(struct clk *clk, const char 
*con_id,
return cl ? 0 : -ENOMEM;
 }
 EXPORT_SYMBOL(clk_register_clkdev);
-
-/**
- * clk_register_clkdevs - register a set of clk_lookup for a struct clk
- * @clk: struct clk to associate with all clk_lookups
- * @cl: array of clk_lookup structures with con_id and dev_id pre-initialized
- * @num: number of clk_lookup structures to register
- *
- * To make things easier for mass registration, we detect error clks
- * from a previous clk_register() call, and return the error code for
- * those.  This is to permit this function to be called immediately
- * after clk_register().
- */
-int clk_register_clkdevs(struct clk *clk, struct clk_lookup *cl, size_t num)
-{
-   unsigned i;
-
-   if (IS_ERR(clk))
-   return PTR_ERR(clk);
-
-   for (i = 0; i < num; i++, cl++) {
-   cl->clk_hw = __clk_get_hw(clk);
-   __clkdev_add(cl);
-   }
-
-   return 0;
-}
-EXPORT_SYMBOL(clk_register_clkdevs);
diff --git a/include/linux/clkdev.h b/include/linux/clkdev.h
index c2c04f7cbe8a..e6f8eb1d585f 100644
--- a/include/linux/clkdev.h
+++ b/include/linux/clkdev.h
@@ -45,7 +45,6 @@ void clkdev_add_table(struct clk_lookup *, size_t);
 int clk_add_alias(const char *, const char *, const char *, struct device *);
 
 int clk_register_clkdev(struct clk *, const char *, const char *);
-int clk_register_clkdevs(struct clk *, struct clk_lookup *, size_t);
 
 #ifdef CONFIG_COMMON_CLK
 int __clk_get(struct clk *clk);
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v2 12/13] clk: fixed-rate: Add hw based registration APIs

[Stephen Boyd]

Add registration APIs in the clk fixed-rate code to return struct
clk_hw pointers instead of struct clk pointers. This way we hide
the struct clk pointer from providers unless they need to use
consumer facing APIs.

Signed-off-by: Stephen Boyd 
---
 drivers/clk/clk-fixed-rate.c | 44 
 include/linux/clk-provider.h |  7 +++
 2 files changed, 43 insertions(+), 8 deletions(-)

diff --git a/drivers/clk/clk-fixed-rate.c b/drivers/clk/clk-fixed-rate.c
index cd9dc925b3f8..8e4453eb54e8 100644
--- a/drivers/clk/clk-fixed-rate.c
+++ b/drivers/clk/clk-fixed-rate.c
@@ -45,8 +45,8 @@ const struct clk_ops clk_fixed_rate_ops = {
 EXPORT_SYMBOL_GPL(clk_fixed_rate_ops);
 
 /**
- * clk_register_fixed_rate_with_accuracy - register fixed-rate clock with the
- *clock framework
+ * clk_hw_register_fixed_rate_with_accuracy - register fixed-rate clock with
+ * the clock framework
  * @dev: device that is registering this clock
  * @name: name of this clock
  * @parent_name: name of clock's parent
@@ -54,13 +54,14 @@ EXPORT_SYMBOL_GPL(clk_fixed_rate_ops);
  * @fixed_rate: non-adjustable clock rate
  * @fixed_accuracy: non-adjustable clock rate
  */
-struct clk *clk_register_fixed_rate_with_accuracy(struct device *dev,
+struct clk_hw *clk_hw_register_fixed_rate_with_accuracy(struct device *dev,
const char *name, const char *parent_name, unsigned long flags,
unsigned long fixed_rate, unsigned long fixed_accuracy)
 {
struct clk_fixed_rate *fixed;
-   struct clk *clk;
+   struct clk_hw *hw;
struct clk_init_data init;
+   int ret;
 
/* allocate fixed-rate clock */
fixed = kzalloc(sizeof(*fixed), GFP_KERNEL);
@@ -79,22 +80,49 @@ struct clk *clk_register_fixed_rate_with_accuracy(struct 
device *dev,
fixed->hw.init = 
 
/* register the clock */
-   clk = clk_register(dev, >hw);
-   if (IS_ERR(clk))
+   hw = >hw;
+   ret = clk_hw_register(dev, hw);
+   if (ret) {
kfree(fixed);
+   hw = ERR_PTR(ret);
+   }
 
-   return clk;
+   return hw;
+}
+EXPORT_SYMBOL_GPL(clk_hw_register_fixed_rate_with_accuracy);
+
+struct clk *clk_register_fixed_rate_with_accuracy(struct device *dev,
+   const char *name, const char *parent_name, unsigned long flags,
+   unsigned long fixed_rate, unsigned long fixed_accuracy)
+{
+   struct clk_hw *hw;
+
+   hw = clk_hw_register_fixed_rate_with_accuracy(dev, name, parent_name,
+   flags, fixed_rate, fixed_accuracy);
+   if (IS_ERR(hw))
+   return ERR_CAST(hw);
+   return hw->clk;
 }
 EXPORT_SYMBOL_GPL(clk_register_fixed_rate_with_accuracy);
 
 /**
- * clk_register_fixed_rate - register fixed-rate clock with the clock framework
+ * clk_hw_register_fixed_rate - register fixed-rate clock with the clock
+ * framework
  * @dev: device that is registering this clock
  * @name: name of this clock
  * @parent_name: name of clock's parent
  * @flags: framework-specific flags
  * @fixed_rate: non-adjustable clock rate
  */
+struct clk_hw *clk_hw_register_fixed_rate(struct device *dev, const char *name,
+   const char *parent_name, unsigned long flags,
+   unsigned long fixed_rate)
+{
+   return clk_hw_register_fixed_rate_with_accuracy(dev, name, parent_name,
+flags, fixed_rate, 0);
+}
+EXPORT_SYMBOL_GPL(clk_hw_register_fixed_rate);
+
 struct clk *clk_register_fixed_rate(struct device *dev, const char *name,
const char *parent_name, unsigned long flags,
unsigned long fixed_rate)
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 6c36c5e8ccbe..c3fc042d517c 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -282,10 +282,17 @@ extern const struct clk_ops clk_fixed_rate_ops;
 struct clk *clk_register_fixed_rate(struct device *dev, const char *name,
const char *parent_name, unsigned long flags,
unsigned long fixed_rate);
+struct clk_hw *clk_hw_register_fixed_rate(struct device *dev, const char *name,
+   const char *parent_name, unsigned long flags,
+   unsigned long fixed_rate);
 struct clk *clk_register_fixed_rate_with_accuracy(struct device *dev,
const char *name, const char *parent_name, unsigned long flags,
unsigned long fixed_rate, unsigned long fixed_accuracy);
 void clk_unregister_fixed_rate(struct clk *clk);
+struct clk_hw *clk_hw_register_fixed_rate_with_accuracy(struct device *dev,
+   const char *name, const char *parent_name, unsigned long flags,
+   unsigned long fixed_rate, unsigned long fixed_accuracy);
+
 void of_fixed_clk_setup(struct device_node *np);
 
 /**
-- 
The Qualcomm Innovation Center, Inc. is a member 

[PATCH v2 08/13] clk: fixed-factor: Add hw based registration APIs

[Stephen Boyd]

Add registration APIs in the clk fixed-factor code to return
struct clk_hw pointers instead of struct clk pointers. This way
we hide the struct clk pointer from providers unless they need to
use consumer facing APIs.

Signed-off-by: Stephen Boyd 
---
 drivers/clk/clk-fixed-factor.c | 42 +++---
 include/linux/clk-provider.h   |  4 
 2 files changed, 39 insertions(+), 7 deletions(-)

diff --git a/drivers/clk/clk-fixed-factor.c b/drivers/clk/clk-fixed-factor.c
index 053448e2453d..75cd6c792cb8 100644
--- a/drivers/clk/clk-fixed-factor.c
+++ b/drivers/clk/clk-fixed-factor.c
@@ -68,13 +68,14 @@ const struct clk_ops clk_fixed_factor_ops = {
 };
 EXPORT_SYMBOL_GPL(clk_fixed_factor_ops);
 
-struct clk *clk_register_fixed_factor(struct device *dev, const char *name,
-   const char *parent_name, unsigned long flags,
+struct clk_hw *clk_hw_register_fixed_factor(struct device *dev,
+   const char *name, const char *parent_name, unsigned long flags,
unsigned int mult, unsigned int div)
 {
struct clk_fixed_factor *fix;
struct clk_init_data init;
-   struct clk *clk;
+   struct clk_hw *hw;
+   int ret;
 
fix = kmalloc(sizeof(*fix), GFP_KERNEL);
if (!fix)
@@ -91,12 +92,28 @@ struct clk *clk_register_fixed_factor(struct device *dev, 
const char *name,
init.parent_names = _name;
init.num_parents = 1;
 
-   clk = clk_register(dev, >hw);
-
-   if (IS_ERR(clk))
+   hw = >hw;
+   ret = clk_hw_register(dev, hw);
+   if (ret) {
kfree(fix);
+   hw = ERR_PTR(ret);
+   }
+
+   return hw;
+}
+EXPORT_SYMBOL_GPL(clk_hw_register_fixed_factor);
+
+struct clk *clk_register_fixed_factor(struct device *dev, const char *name,
+   const char *parent_name, unsigned long flags,
+   unsigned int mult, unsigned int div)
+{
+   struct clk_hw *hw;
 
-   return clk;
+   hw = clk_hw_register_fixed_factor(dev, name, parent_name, flags, mult,
+ div);
+   if (IS_ERR(hw))
+   return ERR_CAST(hw);
+   return hw->clk;
 }
 EXPORT_SYMBOL_GPL(clk_register_fixed_factor);
 
@@ -113,6 +130,17 @@ void clk_unregister_fixed_factor(struct clk *clk)
 }
 EXPORT_SYMBOL_GPL(clk_unregister_fixed_factor);
 
+void clk_hw_unregister_fixed_factor(struct clk_hw *hw)
+{
+   struct clk_fixed_factor *fix;
+
+   fix = to_clk_fixed_factor(hw);
+
+   clk_hw_unregister(hw);
+   kfree(fix);
+}
+EXPORT_SYMBOL_GPL(clk_hw_unregister_fixed_factor);
+
 #ifdef CONFIG_OF
 /**
  * of_fixed_factor_clk_setup() - Setup function for simple fixed factor clock
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index d690d99b9c1c..79ad1a8a6831 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -525,6 +525,10 @@ struct clk *clk_register_fixed_factor(struct device *dev, 
const char *name,
const char *parent_name, unsigned long flags,
unsigned int mult, unsigned int div);
 void clk_unregister_fixed_factor(struct clk *clk);
+struct clk_hw *clk_hw_register_fixed_factor(struct device *dev,
+   const char *name, const char *parent_name, unsigned long flags,
+   unsigned int mult, unsigned int div);
+void clk_hw_unregister_fixed_factor(struct clk_hw *hw);
 
 /**
  * struct clk_fractional_divider - adjustable fractional divider clock
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v2 01/13] clkdev: Remove clk_register_clkdevs()

[Stephen Boyd]

Now that we've converted the only caller over to another clkdev
API, remove this one.

Reviewed-by: Andy Shevchenko 
Cc: Russell King 
Signed-off-by: Stephen Boyd 
---
 drivers/clk/clkdev.c   | 27 ---
 include/linux/clkdev.h |  1 -
 2 files changed, 28 deletions(-)

diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c
index eb20b941154b..ae8e40a82d34 100644
--- a/drivers/clk/clkdev.c
+++ b/drivers/clk/clkdev.c
@@ -402,30 +402,3 @@ int clk_register_clkdev(struct clk *clk, const char 
*con_id,
return cl ? 0 : -ENOMEM;
 }
 EXPORT_SYMBOL(clk_register_clkdev);
-
-/**
- * clk_register_clkdevs - register a set of clk_lookup for a struct clk
- * @clk: struct clk to associate with all clk_lookups
- * @cl: array of clk_lookup structures with con_id and dev_id pre-initialized
- * @num: number of clk_lookup structures to register
- *
- * To make things easier for mass registration, we detect error clks
- * from a previous clk_register() call, and return the error code for
- * those.  This is to permit this function to be called immediately
- * after clk_register().
- */
-int clk_register_clkdevs(struct clk *clk, struct clk_lookup *cl, size_t num)
-{
-   unsigned i;
-
-   if (IS_ERR(clk))
-   return PTR_ERR(clk);
-
-   for (i = 0; i < num; i++, cl++) {
-   cl->clk_hw = __clk_get_hw(clk);
-   __clkdev_add(cl);
-   }
-
-   return 0;
-}
-EXPORT_SYMBOL(clk_register_clkdevs);
diff --git a/include/linux/clkdev.h b/include/linux/clkdev.h
index c2c04f7cbe8a..e6f8eb1d585f 100644
--- a/include/linux/clkdev.h
+++ b/include/linux/clkdev.h
@@ -45,7 +45,6 @@ void clkdev_add_table(struct clk_lookup *, size_t);
 int clk_add_alias(const char *, const char *, const char *, struct device *);
 
 int clk_register_clkdev(struct clk *, const char *, const char *);
-int clk_register_clkdevs(struct clk *, struct clk_lookup *, size_t);
 
 #ifdef CONFIG_COMMON_CLK
 int __clk_get(struct clk *clk);
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v2 12/13] clk: fixed-rate: Add hw based registration APIs

[Stephen Boyd]

Add registration APIs in the clk fixed-rate code to return struct
clk_hw pointers instead of struct clk pointers. This way we hide
the struct clk pointer from providers unless they need to use
consumer facing APIs.

Signed-off-by: Stephen Boyd 
---
 drivers/clk/clk-fixed-rate.c | 44 
 include/linux/clk-provider.h |  7 +++
 2 files changed, 43 insertions(+), 8 deletions(-)

diff --git a/drivers/clk/clk-fixed-rate.c b/drivers/clk/clk-fixed-rate.c
index cd9dc925b3f8..8e4453eb54e8 100644
--- a/drivers/clk/clk-fixed-rate.c
+++ b/drivers/clk/clk-fixed-rate.c
@@ -45,8 +45,8 @@ const struct clk_ops clk_fixed_rate_ops = {
 EXPORT_SYMBOL_GPL(clk_fixed_rate_ops);
 
 /**
- * clk_register_fixed_rate_with_accuracy - register fixed-rate clock with the
- *clock framework
+ * clk_hw_register_fixed_rate_with_accuracy - register fixed-rate clock with
+ * the clock framework
  * @dev: device that is registering this clock
  * @name: name of this clock
  * @parent_name: name of clock's parent
@@ -54,13 +54,14 @@ EXPORT_SYMBOL_GPL(clk_fixed_rate_ops);
  * @fixed_rate: non-adjustable clock rate
  * @fixed_accuracy: non-adjustable clock rate
  */
-struct clk *clk_register_fixed_rate_with_accuracy(struct device *dev,
+struct clk_hw *clk_hw_register_fixed_rate_with_accuracy(struct device *dev,
const char *name, const char *parent_name, unsigned long flags,
unsigned long fixed_rate, unsigned long fixed_accuracy)
 {
struct clk_fixed_rate *fixed;
-   struct clk *clk;
+   struct clk_hw *hw;
struct clk_init_data init;
+   int ret;
 
/* allocate fixed-rate clock */
fixed = kzalloc(sizeof(*fixed), GFP_KERNEL);
@@ -79,22 +80,49 @@ struct clk *clk_register_fixed_rate_with_accuracy(struct 
device *dev,
fixed->hw.init = 
 
/* register the clock */
-   clk = clk_register(dev, >hw);
-   if (IS_ERR(clk))
+   hw = >hw;
+   ret = clk_hw_register(dev, hw);
+   if (ret) {
kfree(fixed);
+   hw = ERR_PTR(ret);
+   }
 
-   return clk;
+   return hw;
+}
+EXPORT_SYMBOL_GPL(clk_hw_register_fixed_rate_with_accuracy);
+
+struct clk *clk_register_fixed_rate_with_accuracy(struct device *dev,
+   const char *name, const char *parent_name, unsigned long flags,
+   unsigned long fixed_rate, unsigned long fixed_accuracy)
+{
+   struct clk_hw *hw;
+
+   hw = clk_hw_register_fixed_rate_with_accuracy(dev, name, parent_name,
+   flags, fixed_rate, fixed_accuracy);
+   if (IS_ERR(hw))
+   return ERR_CAST(hw);
+   return hw->clk;
 }
 EXPORT_SYMBOL_GPL(clk_register_fixed_rate_with_accuracy);
 
 /**
- * clk_register_fixed_rate - register fixed-rate clock with the clock framework
+ * clk_hw_register_fixed_rate - register fixed-rate clock with the clock
+ * framework
  * @dev: device that is registering this clock
  * @name: name of this clock
  * @parent_name: name of clock's parent
  * @flags: framework-specific flags
  * @fixed_rate: non-adjustable clock rate
  */
+struct clk_hw *clk_hw_register_fixed_rate(struct device *dev, const char *name,
+   const char *parent_name, unsigned long flags,
+   unsigned long fixed_rate)
+{
+   return clk_hw_register_fixed_rate_with_accuracy(dev, name, parent_name,
+flags, fixed_rate, 0);
+}
+EXPORT_SYMBOL_GPL(clk_hw_register_fixed_rate);
+
 struct clk *clk_register_fixed_rate(struct device *dev, const char *name,
const char *parent_name, unsigned long flags,
unsigned long fixed_rate)
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 6c36c5e8ccbe..c3fc042d517c 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -282,10 +282,17 @@ extern const struct clk_ops clk_fixed_rate_ops;
 struct clk *clk_register_fixed_rate(struct device *dev, const char *name,
const char *parent_name, unsigned long flags,
unsigned long fixed_rate);
+struct clk_hw *clk_hw_register_fixed_rate(struct device *dev, const char *name,
+   const char *parent_name, unsigned long flags,
+   unsigned long fixed_rate);
 struct clk *clk_register_fixed_rate_with_accuracy(struct device *dev,
const char *name, const char *parent_name, unsigned long flags,
unsigned long fixed_rate, unsigned long fixed_accuracy);
 void clk_unregister_fixed_rate(struct clk *clk);
+struct clk_hw *clk_hw_register_fixed_rate_with_accuracy(struct device *dev,
+   const char *name, const char *parent_name, unsigned long flags,
+   unsigned long fixed_rate, unsigned long fixed_accuracy);
+
 void of_fixed_clk_setup(struct device_node *np);
 
 /**
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 

[PATCH v2 08/13] clk: fixed-factor: Add hw based registration APIs

[Stephen Boyd]

Add registration APIs in the clk fixed-factor code to return
struct clk_hw pointers instead of struct clk pointers. This way
we hide the struct clk pointer from providers unless they need to
use consumer facing APIs.

Signed-off-by: Stephen Boyd 
---
 drivers/clk/clk-fixed-factor.c | 42 +++---
 include/linux/clk-provider.h   |  4 
 2 files changed, 39 insertions(+), 7 deletions(-)

diff --git a/drivers/clk/clk-fixed-factor.c b/drivers/clk/clk-fixed-factor.c
index 053448e2453d..75cd6c792cb8 100644
--- a/drivers/clk/clk-fixed-factor.c
+++ b/drivers/clk/clk-fixed-factor.c
@@ -68,13 +68,14 @@ const struct clk_ops clk_fixed_factor_ops = {
 };
 EXPORT_SYMBOL_GPL(clk_fixed_factor_ops);
 
-struct clk *clk_register_fixed_factor(struct device *dev, const char *name,
-   const char *parent_name, unsigned long flags,
+struct clk_hw *clk_hw_register_fixed_factor(struct device *dev,
+   const char *name, const char *parent_name, unsigned long flags,
unsigned int mult, unsigned int div)
 {
struct clk_fixed_factor *fix;
struct clk_init_data init;
-   struct clk *clk;
+   struct clk_hw *hw;
+   int ret;
 
fix = kmalloc(sizeof(*fix), GFP_KERNEL);
if (!fix)
@@ -91,12 +92,28 @@ struct clk *clk_register_fixed_factor(struct device *dev, 
const char *name,
init.parent_names = _name;
init.num_parents = 1;
 
-   clk = clk_register(dev, >hw);
-
-   if (IS_ERR(clk))
+   hw = >hw;
+   ret = clk_hw_register(dev, hw);
+   if (ret) {
kfree(fix);
+   hw = ERR_PTR(ret);
+   }
+
+   return hw;
+}
+EXPORT_SYMBOL_GPL(clk_hw_register_fixed_factor);
+
+struct clk *clk_register_fixed_factor(struct device *dev, const char *name,
+   const char *parent_name, unsigned long flags,
+   unsigned int mult, unsigned int div)
+{
+   struct clk_hw *hw;
 
-   return clk;
+   hw = clk_hw_register_fixed_factor(dev, name, parent_name, flags, mult,
+ div);
+   if (IS_ERR(hw))
+   return ERR_CAST(hw);
+   return hw->clk;
 }
 EXPORT_SYMBOL_GPL(clk_register_fixed_factor);
 
@@ -113,6 +130,17 @@ void clk_unregister_fixed_factor(struct clk *clk)
 }
 EXPORT_SYMBOL_GPL(clk_unregister_fixed_factor);
 
+void clk_hw_unregister_fixed_factor(struct clk_hw *hw)
+{
+   struct clk_fixed_factor *fix;
+
+   fix = to_clk_fixed_factor(hw);
+
+   clk_hw_unregister(hw);
+   kfree(fix);
+}
+EXPORT_SYMBOL_GPL(clk_hw_unregister_fixed_factor);
+
 #ifdef CONFIG_OF
 /**
  * of_fixed_factor_clk_setup() - Setup function for simple fixed factor clock
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index d690d99b9c1c..79ad1a8a6831 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -525,6 +525,10 @@ struct clk *clk_register_fixed_factor(struct device *dev, 
const char *name,
const char *parent_name, unsigned long flags,
unsigned int mult, unsigned int div);
 void clk_unregister_fixed_factor(struct clk *clk);
+struct clk_hw *clk_hw_register_fixed_factor(struct device *dev,
+   const char *name, const char *parent_name, unsigned long flags,
+   unsigned int mult, unsigned int div);
+void clk_hw_unregister_fixed_factor(struct clk_hw *hw);
 
 /**
  * struct clk_fractional_divider - adjustable fractional divider clock
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v2 11/13] clk: gpio: Add hw based registration APIs

[Stephen Boyd]

Add registration APIs in the clk gpio code to return struct
clk_hw pointers instead of struct clk pointers. This way we hide
the struct clk pointer from providers unless they need to use
consumer facing APIs.

Signed-off-by: Stephen Boyd 
---
 drivers/clk/clk-gpio.c   | 52 ++--
 include/linux/clk-provider.h |  8 +++
 2 files changed, 49 insertions(+), 11 deletions(-)

diff --git a/drivers/clk/clk-gpio.c b/drivers/clk/clk-gpio.c
index 08f65acc5d57..86b245746a6b 100644
--- a/drivers/clk/clk-gpio.c
+++ b/drivers/clk/clk-gpio.c
@@ -94,13 +94,13 @@ const struct clk_ops clk_gpio_mux_ops = {
 };
 EXPORT_SYMBOL_GPL(clk_gpio_mux_ops);
 
-static struct clk *clk_register_gpio(struct device *dev, const char *name,
+static struct clk_hw *clk_register_gpio(struct device *dev, const char *name,
const char * const *parent_names, u8 num_parents, unsigned gpio,
bool active_low, unsigned long flags,
const struct clk_ops *clk_gpio_ops)
 {
struct clk_gpio *clk_gpio;
-   struct clk *clk;
+   struct clk_hw *hw;
struct clk_init_data init = {};
unsigned long gpio_flags;
int err;
@@ -141,24 +141,26 @@ static struct clk *clk_register_gpio(struct device *dev, 
const char *name,
clk_gpio->gpiod = gpio_to_desc(gpio);
clk_gpio->hw.init = 
 
+   hw = _gpio->hw;
if (dev)
-   clk = devm_clk_register(dev, _gpio->hw);
+   err = devm_clk_hw_register(dev, hw);
else
-   clk = clk_register(NULL, _gpio->hw);
+   err = clk_hw_register(NULL, hw);
 
-   if (!IS_ERR(clk))
-   return clk;
+   if (!err)
+   return hw;
 
if (!dev) {
gpiod_put(clk_gpio->gpiod);
kfree(clk_gpio);
}
 
-   return clk;
+   return ERR_PTR(err);
 }
 
 /**
- * clk_register_gpio_gate - register a gpio clock gate with the clock framework
+ * clk_hw_register_gpio_gate - register a gpio clock gate with the clock
+ * framework
  * @dev: device that is registering this clock
  * @name: name of this clock
  * @parent_name: name of this clock's parent
@@ -166,7 +168,7 @@ static struct clk *clk_register_gpio(struct device *dev, 
const char *name,
  * @active_low: true if gpio should be set to 0 to enable clock
  * @flags: clock flags
  */
-struct clk *clk_register_gpio_gate(struct device *dev, const char *name,
+struct clk_hw *clk_hw_register_gpio_gate(struct device *dev, const char *name,
const char *parent_name, unsigned gpio, bool active_low,
unsigned long flags)
 {
@@ -175,10 +177,24 @@ struct clk *clk_register_gpio_gate(struct device *dev, 
const char *name,
(parent_name ? 1 : 0), gpio, active_low, flags,
_gpio_gate_ops);
 }
+EXPORT_SYMBOL_GPL(clk_hw_register_gpio_gate);
+
+struct clk *clk_register_gpio_gate(struct device *dev, const char *name,
+   const char *parent_name, unsigned gpio, bool active_low,
+   unsigned long flags)
+{
+   struct clk_hw *hw;
+
+   hw = clk_hw_register_gpio_gate(dev, name, parent_name, gpio, active_low,
+  flags);
+   if (IS_ERR(hw))
+   return ERR_CAST(hw);
+   return hw->clk;
+}
 EXPORT_SYMBOL_GPL(clk_register_gpio_gate);
 
 /**
- * clk_register_gpio_mux - register a gpio clock mux with the clock framework
+ * clk_hw_register_gpio_mux - register a gpio clock mux with the clock 
framework
  * @dev: device that is registering this clock
  * @name: name of this clock
  * @parent_names: names of this clock's parents
@@ -187,7 +203,7 @@ EXPORT_SYMBOL_GPL(clk_register_gpio_gate);
  * @active_low: true if gpio should be set to 0 to enable clock
  * @flags: clock flags
  */
-struct clk *clk_register_gpio_mux(struct device *dev, const char *name,
+struct clk_hw *clk_hw_register_gpio_mux(struct device *dev, const char *name,
const char * const *parent_names, u8 num_parents, unsigned gpio,
bool active_low, unsigned long flags)
 {
@@ -199,6 +215,20 @@ struct clk *clk_register_gpio_mux(struct device *dev, 
const char *name,
return clk_register_gpio(dev, name, parent_names, num_parents,
gpio, active_low, flags, _gpio_mux_ops);
 }
+EXPORT_SYMBOL_GPL(clk_hw_register_gpio_mux);
+
+struct clk *clk_register_gpio_mux(struct device *dev, const char *name,
+   const char * const *parent_names, u8 num_parents, unsigned gpio,
+   bool active_low, unsigned long flags)
+{
+   struct clk_hw *hw;
+
+   hw = clk_hw_register_gpio_mux(dev, name, parent_names, num_parents,
+   gpio, active_low, flags);
+   if (IS_ERR(hw))
+   return ERR_CAST(hw);
+   return hw->clk;
+}
 EXPORT_SYMBOL_GPL(clk_register_gpio_mux);
 
 static int gpio_clk_driver_probe(struct 

[PATCH v2 11/13] clk: gpio: Add hw based registration APIs

[Stephen Boyd]

Add registration APIs in the clk gpio code to return struct
clk_hw pointers instead of struct clk pointers. This way we hide
the struct clk pointer from providers unless they need to use
consumer facing APIs.

Signed-off-by: Stephen Boyd 
---
 drivers/clk/clk-gpio.c   | 52 ++--
 include/linux/clk-provider.h |  8 +++
 2 files changed, 49 insertions(+), 11 deletions(-)

diff --git a/drivers/clk/clk-gpio.c b/drivers/clk/clk-gpio.c
index 08f65acc5d57..86b245746a6b 100644
--- a/drivers/clk/clk-gpio.c
+++ b/drivers/clk/clk-gpio.c
@@ -94,13 +94,13 @@ const struct clk_ops clk_gpio_mux_ops = {
 };
 EXPORT_SYMBOL_GPL(clk_gpio_mux_ops);
 
-static struct clk *clk_register_gpio(struct device *dev, const char *name,
+static struct clk_hw *clk_register_gpio(struct device *dev, const char *name,
const char * const *parent_names, u8 num_parents, unsigned gpio,
bool active_low, unsigned long flags,
const struct clk_ops *clk_gpio_ops)
 {
struct clk_gpio *clk_gpio;
-   struct clk *clk;
+   struct clk_hw *hw;
struct clk_init_data init = {};
unsigned long gpio_flags;
int err;
@@ -141,24 +141,26 @@ static struct clk *clk_register_gpio(struct device *dev, 
const char *name,
clk_gpio->gpiod = gpio_to_desc(gpio);
clk_gpio->hw.init = 
 
+   hw = _gpio->hw;
if (dev)
-   clk = devm_clk_register(dev, _gpio->hw);
+   err = devm_clk_hw_register(dev, hw);
else
-   clk = clk_register(NULL, _gpio->hw);
+   err = clk_hw_register(NULL, hw);
 
-   if (!IS_ERR(clk))
-   return clk;
+   if (!err)
+   return hw;
 
if (!dev) {
gpiod_put(clk_gpio->gpiod);
kfree(clk_gpio);
}
 
-   return clk;
+   return ERR_PTR(err);
 }
 
 /**
- * clk_register_gpio_gate - register a gpio clock gate with the clock framework
+ * clk_hw_register_gpio_gate - register a gpio clock gate with the clock
+ * framework
  * @dev: device that is registering this clock
  * @name: name of this clock
  * @parent_name: name of this clock's parent
@@ -166,7 +168,7 @@ static struct clk *clk_register_gpio(struct device *dev, 
const char *name,
  * @active_low: true if gpio should be set to 0 to enable clock
  * @flags: clock flags
  */
-struct clk *clk_register_gpio_gate(struct device *dev, const char *name,
+struct clk_hw *clk_hw_register_gpio_gate(struct device *dev, const char *name,
const char *parent_name, unsigned gpio, bool active_low,
unsigned long flags)
 {
@@ -175,10 +177,24 @@ struct clk *clk_register_gpio_gate(struct device *dev, 
const char *name,
(parent_name ? 1 : 0), gpio, active_low, flags,
_gpio_gate_ops);
 }
+EXPORT_SYMBOL_GPL(clk_hw_register_gpio_gate);
+
+struct clk *clk_register_gpio_gate(struct device *dev, const char *name,
+   const char *parent_name, unsigned gpio, bool active_low,
+   unsigned long flags)
+{
+   struct clk_hw *hw;
+
+   hw = clk_hw_register_gpio_gate(dev, name, parent_name, gpio, active_low,
+  flags);
+   if (IS_ERR(hw))
+   return ERR_CAST(hw);
+   return hw->clk;
+}
 EXPORT_SYMBOL_GPL(clk_register_gpio_gate);
 
 /**
- * clk_register_gpio_mux - register a gpio clock mux with the clock framework
+ * clk_hw_register_gpio_mux - register a gpio clock mux with the clock 
framework
  * @dev: device that is registering this clock
  * @name: name of this clock
  * @parent_names: names of this clock's parents
@@ -187,7 +203,7 @@ EXPORT_SYMBOL_GPL(clk_register_gpio_gate);
  * @active_low: true if gpio should be set to 0 to enable clock
  * @flags: clock flags
  */
-struct clk *clk_register_gpio_mux(struct device *dev, const char *name,
+struct clk_hw *clk_hw_register_gpio_mux(struct device *dev, const char *name,
const char * const *parent_names, u8 num_parents, unsigned gpio,
bool active_low, unsigned long flags)
 {
@@ -199,6 +215,20 @@ struct clk *clk_register_gpio_mux(struct device *dev, 
const char *name,
return clk_register_gpio(dev, name, parent_names, num_parents,
gpio, active_low, flags, _gpio_mux_ops);
 }
+EXPORT_SYMBOL_GPL(clk_hw_register_gpio_mux);
+
+struct clk *clk_register_gpio_mux(struct device *dev, const char *name,
+   const char * const *parent_names, u8 num_parents, unsigned gpio,
+   bool active_low, unsigned long flags)
+{
+   struct clk_hw *hw;
+
+   hw = clk_hw_register_gpio_mux(dev, name, parent_names, num_parents,
+   gpio, active_low, flags);
+   if (IS_ERR(hw))
+   return ERR_CAST(hw);
+   return hw->clk;
+}
 EXPORT_SYMBOL_GPL(clk_register_gpio_mux);
 
 static int gpio_clk_driver_probe(struct platform_device *pdev)
diff 

[PATCH v2 02/13] clk: Add {devm_}clk_hw_{register,unregister}() APIs

[Stephen Boyd]

We've largely split the clk consumer and provider APIs along
struct clk and struct clk_hw, but clk_register() still returns a
struct clk pointer for each struct clk_hw that's registered.
Eventually we'd like to only allocate struct clks when there's a
user, because struct clk is per-user now, so clk_register() needs
to change.

Let's add new APIs to register struct clk_hws, but this time
we'll hide the struct clk from the caller by returning an int
error code. Also add an unregistration API that takes the clk_hw
structure that was passed to the registration API. This way
provider drivers never have to deal with a struct clk pointer
unless they're using the clk consumer APIs.

Signed-off-by: Stephen Boyd 
---
 Documentation/driver-model/devres.txt |  1 +
 drivers/clk/clk.c | 86 +++
 include/linux/clk-provider.h  |  6 +++
 3 files changed, 93 insertions(+)

diff --git a/Documentation/driver-model/devres.txt 
b/Documentation/driver-model/devres.txt
index 73b98dfbcea4..108d45553e1b 100644
--- a/Documentation/driver-model/devres.txt
+++ b/Documentation/driver-model/devres.txt
@@ -236,6 +236,7 @@ certainly invest a bit more effort into libata core layer).
 CLOCK
   devm_clk_get()
   devm_clk_put()
+  devm_clk_hw_register()
 
 DMA
   dmam_alloc_coherent()
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index fb74dc1f7520..0ef919666827 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2536,6 +2536,22 @@ fail_out:
 }
 EXPORT_SYMBOL_GPL(clk_register);
 
+/**
+ * clk_hw_register - register a clk_hw and return an error code
+ * @dev: device that is registering this clock
+ * @hw: link to hardware-specific clock data
+ *
+ * clk_hw_register is the primary interface for populating the clock tree with
+ * new clock nodes. It returns an integer equal to zero indicating success or
+ * less than zero indicating failure. Drivers must test for an error code after
+ * calling clk_hw_register().
+ */
+int clk_hw_register(struct device *dev, struct clk_hw *hw)
+{
+   return PTR_ERR_OR_ZERO(clk_register(dev, hw));
+}
+EXPORT_SYMBOL_GPL(clk_hw_register);
+
 /* Free memory allocated for a clock. */
 static void __clk_release(struct kref *ref)
 {
@@ -2637,11 +2653,26 @@ unlock:
 }
 EXPORT_SYMBOL_GPL(clk_unregister);
 
+/**
+ * clk_hw_unregister - unregister a currently registered clk_hw
+ * @hw: hardware-specific clock data to unregister
+ */
+void clk_hw_unregister(struct clk_hw *hw)
+{
+   clk_unregister(hw->clk);
+}
+EXPORT_SYMBOL_GPL(clk_hw_unregister);
+
 static void devm_clk_release(struct device *dev, void *res)
 {
clk_unregister(*(struct clk **)res);
 }
 
+static void devm_clk_hw_release(struct device *dev, void *res)
+{
+   clk_hw_unregister(*(struct clk_hw **)res);
+}
+
 /**
  * devm_clk_register - resource managed clk_register()
  * @dev: device that is registering this clock
@@ -2672,6 +2703,36 @@ struct clk *devm_clk_register(struct device *dev, struct 
clk_hw *hw)
 }
 EXPORT_SYMBOL_GPL(devm_clk_register);
 
+/**
+ * devm_clk_hw_register - resource managed clk_hw_register()
+ * @dev: device that is registering this clock
+ * @hw: link to hardware-specific clock data
+ *
+ * Managed clk_hw_register(). Clocks returned from this function are
+ * automatically clk_hw_unregister()ed on driver detach. See clk_hw_register()
+ * for more information.
+ */
+int devm_clk_hw_register(struct device *dev, struct clk_hw *hw)
+{
+   struct clk_hw **hwp;
+   int ret;
+
+   hwp = devres_alloc(devm_clk_hw_release, sizeof(*hwp), GFP_KERNEL);
+   if (!hwp)
+   return -ENOMEM;
+
+   ret = clk_hw_register(dev, hw);
+   if (!ret) {
+   *hwp = hw;
+   devres_add(dev, hwp);
+   } else {
+   devres_free(hwp);
+   }
+
+   return ret;
+}
+EXPORT_SYMBOL_GPL(devm_clk_hw_register);
+
 static int devm_clk_match(struct device *dev, void *res, void *data)
 {
struct clk *c = res;
@@ -2680,6 +2741,15 @@ static int devm_clk_match(struct device *dev, void *res, 
void *data)
return c == data;
 }
 
+static int devm_clk_hw_match(struct device *dev, void *res, void *data)
+{
+   struct clk_hw *hw = res;
+
+   if (WARN_ON(!hw))
+   return 0;
+   return hw == data;
+}
+
 /**
  * devm_clk_unregister - resource managed clk_unregister()
  * @clk: clock to unregister
@@ -2694,6 +2764,22 @@ void devm_clk_unregister(struct device *dev, struct clk 
*clk)
 }
 EXPORT_SYMBOL_GPL(devm_clk_unregister);
 
+/**
+ * devm_clk_hw_unregister - resource managed clk_hw_unregister()
+ * @dev: device that is unregistering the hardware-specific clock data
+ * @hw: link to hardware-specific clock data
+ *
+ * Unregister a clk_hw registered with devm_clk_hw_register(). Normally
+ * this function will not need to be called and the resource management
+ * code will ensure that the resource is freed.
+ */
+void devm_clk_hw_unregister(struct device *dev, 

[PATCH v2 00/13] clk_hw based clkdev/DT providers

[Stephen Boyd]

We've mostly split the clk API between consumers and providers
along struct clk and struct clk_hw, but the registration and
clkdev/DT code is still struct clk focused. This series
lays the foundation for changing that design by making
clk_register() return an int instead of a struct clk and
clkdev/DT lookups take a struct clk_hw instead of a struct clk.

After this series is applied, we can avoid using struct clk
in provider drivers entirely, unless we want to use the consumer
APIs. There are quite a few registration callers, so this change
will require converting all of them. I plan to do that gradually
over the coming weeks, but I've included a change to qcom
platforms because I could easily test it and show how things
will work.

The first patch is a cleanup to the clkdev APIs that I found
while doing this work. I'd like to take that into clk-next
with Russell's ack.

Cc: Russell King 

Stephen Boyd (13):
  clkdev: Remove clk_register_clkdevs()
  clk: Add {devm_}clk_hw_{register,unregister}() APIs
  clk: Add clk_hw OF clk providers
  clkdev: Add clk_hw based registration APIs
  clk: divider: Add hw based registration APIs
  clk: gate: Add hw based registration APIs
  clk: mux: Add hw based registration APIs
  clk: fixed-factor: Add hw based registration APIs
  clk: fractional-divider: Add hw based registration APIs
  clk: composite: Add hw based registration APIs
  clk: gpio: Add hw based registration APIs
  clk: fixed-rate: Add hw based registration APIs
  clk: qcom: Migrate to clk_hw based registration and OF APIs

 Documentation/driver-model/devres.txt |   1 +
 drivers/clk/clk-composite.c   |  45 ++---
 drivers/clk/clk-divider.c |  91 --
 drivers/clk/clk-fixed-factor.c|  42 +++--
 drivers/clk/clk-fixed-rate.c  |  44 +++--
 drivers/clk/clk-fractional-divider.c  |  40 +++-
 drivers/clk/clk-gate.c|  43 +++--
 drivers/clk/clk-gpio.c|  52 ---
 drivers/clk/clk-mux.c |  57 ++--
 drivers/clk/clk.c | 171 +-
 drivers/clk/clkdev.c  |  75 +++
 drivers/clk/qcom/clk-regmap.c |   5 +-
 drivers/clk/qcom/clk-regmap.h |   3 +-
 drivers/clk/qcom/common.c |  58 ++--
 drivers/clk/qcom/gcc-msm8996.c|   9 +-
 drivers/clk/qcom/mmcc-msm8996.c   |   9 +-
 include/linux/clk-provider.h  |  93 ++
 include/linux/clkdev.h|   7 +-
 18 files changed, 716 insertions(+), 129 deletions(-)

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v2 10/13] clk: composite: Add hw based registration APIs

[Stephen Boyd]

Add registration APIs in the clk composite code to return struct
clk_hw pointers instead of struct clk pointers. This way we hide
the struct clk pointer from providers unless they need to use
consumer facing APIs.

Signed-off-by: Stephen Boyd 
---
 drivers/clk/clk-composite.c  | 45 
 include/linux/clk-provider.h |  7 +++
 2 files changed, 40 insertions(+), 12 deletions(-)

diff --git a/drivers/clk/clk-composite.c b/drivers/clk/clk-composite.c
index 1f903e1f86a2..463fadd5a68f 100644
--- a/drivers/clk/clk-composite.c
+++ b/drivers/clk/clk-composite.c
@@ -184,17 +184,18 @@ static void clk_composite_disable(struct clk_hw *hw)
gate_ops->disable(gate_hw);
 }
 
-struct clk *clk_register_composite(struct device *dev, const char *name,
+struct clk_hw *clk_hw_register_composite(struct device *dev, const char *name,
const char * const *parent_names, int num_parents,
struct clk_hw *mux_hw, const struct clk_ops *mux_ops,
struct clk_hw *rate_hw, const struct clk_ops *rate_ops,
struct clk_hw *gate_hw, const struct clk_ops *gate_ops,
unsigned long flags)
 {
-   struct clk *clk;
+   struct clk_hw *hw;
struct clk_init_data init;
struct clk_composite *composite;
struct clk_ops *clk_composite_ops;
+   int ret;
 
composite = kzalloc(sizeof(*composite), GFP_KERNEL);
if (!composite)
@@ -204,12 +205,13 @@ struct clk *clk_register_composite(struct device *dev, 
const char *name,
init.flags = flags | CLK_IS_BASIC;
init.parent_names = parent_names;
init.num_parents = num_parents;
+   hw = >hw;
 
clk_composite_ops = >ops;
 
if (mux_hw && mux_ops) {
if (!mux_ops->get_parent) {
-   clk = ERR_PTR(-EINVAL);
+   hw = ERR_PTR(-EINVAL);
goto err;
}
 
@@ -224,7 +226,7 @@ struct clk *clk_register_composite(struct device *dev, 
const char *name,
 
if (rate_hw && rate_ops) {
if (!rate_ops->recalc_rate) {
-   clk = ERR_PTR(-EINVAL);
+   hw = ERR_PTR(-EINVAL);
goto err;
}
clk_composite_ops->recalc_rate = clk_composite_recalc_rate;
@@ -253,7 +255,7 @@ struct clk *clk_register_composite(struct device *dev, 
const char *name,
if (gate_hw && gate_ops) {
if (!gate_ops->is_enabled || !gate_ops->enable ||
!gate_ops->disable) {
-   clk = ERR_PTR(-EINVAL);
+   hw = ERR_PTR(-EINVAL);
goto err;
}
 
@@ -267,22 +269,41 @@ struct clk *clk_register_composite(struct device *dev, 
const char *name,
init.ops = clk_composite_ops;
composite->hw.init = 
 
-   clk = clk_register(dev, >hw);
-   if (IS_ERR(clk))
+   ret = clk_hw_register(dev, hw);
+   if (ret) {
+   hw = ERR_PTR(ret);
goto err;
+   }
 
if (composite->mux_hw)
-   composite->mux_hw->clk = clk;
+   composite->mux_hw->clk = hw->clk;
 
if (composite->rate_hw)
-   composite->rate_hw->clk = clk;
+   composite->rate_hw->clk = hw->clk;
 
if (composite->gate_hw)
-   composite->gate_hw->clk = clk;
+   composite->gate_hw->clk = hw->clk;
 
-   return clk;
+   return hw;
 
 err:
kfree(composite);
-   return clk;
+   return hw;
+}
+
+struct clk *clk_register_composite(struct device *dev, const char *name,
+   const char * const *parent_names, int num_parents,
+   struct clk_hw *mux_hw, const struct clk_ops *mux_ops,
+   struct clk_hw *rate_hw, const struct clk_ops *rate_ops,
+   struct clk_hw *gate_hw, const struct clk_ops *gate_ops,
+   unsigned long flags)
+{
+   struct clk_hw *hw;
+
+   hw = clk_hw_register_composite(dev, name, parent_names, num_parents,
+   mux_hw, mux_ops, rate_hw, rate_ops, gate_hw, gate_ops,
+   flags);
+   if (IS_ERR(hw))
+   return ERR_CAST(hw);
+   return hw->clk;
 }
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index bcbaf6c95d52..456c3ced1ac9 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -638,6 +638,13 @@ struct clk *clk_register_composite(struct device *dev, 
const char *name,
struct clk_hw *rate_hw, const struct clk_ops *rate_ops,
struct clk_hw *gate_hw, const struct clk_ops *gate_ops,
unsigned long flags);
+struct clk_hw *clk_hw_register_composite(struct device *dev, const char *name,
+   const char 

[PATCH v2 04/13] clkdev: Add clk_hw based registration APIs

[Stephen Boyd]

Now that we have a clk registration API that doesn't return
struct clks, we need to have some way to hand out struct clks via
the clk_get() APIs that doesn't involve associating struct clk
pointers with a struct clk_lookup. Luckily, clkdev already
operates on struct clk_hw pointers, except for the registration
facing APIs where it converts struct clk pointers into struct
clk_hw pointers almost immediately.

Let's add clk_hw based registration APIs so that we can skip the
conversion step and provide a way for clk provider drivers to
operate exclusively on clk_hw structs. This way we clearly
split the API between consumers and providers.

Cc: Russell King 
Signed-off-by: Stephen Boyd 
---
 drivers/clk/clkdev.c   | 64 ++
 include/linux/clkdev.h |  6 +
 2 files changed, 70 insertions(+)

diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c
index ae8e40a82d34..89cc700fbc37 100644
--- a/drivers/clk/clkdev.c
+++ b/drivers/clk/clkdev.c
@@ -301,6 +301,20 @@ clkdev_alloc(struct clk *clk, const char *con_id, const 
char *dev_fmt, ...)
 }
 EXPORT_SYMBOL(clkdev_alloc);
 
+struct clk_lookup *
+clkdev_hw_alloc(struct clk_hw *hw, const char *con_id, const char *dev_fmt, 
...)
+{
+   struct clk_lookup *cl;
+   va_list ap;
+
+   va_start(ap, dev_fmt);
+   cl = vclkdev_alloc(hw, con_id, dev_fmt, ap);
+   va_end(ap);
+
+   return cl;
+}
+EXPORT_SYMBOL(clkdev_hw_alloc);
+
 /**
  * clkdev_create - allocate and add a clkdev lookup structure
  * @clk: struct clk to associate with all clk_lookups
@@ -324,6 +338,29 @@ struct clk_lookup *clkdev_create(struct clk *clk, const 
char *con_id,
 }
 EXPORT_SYMBOL_GPL(clkdev_create);
 
+/**
+ * clkdev_hw_create - allocate and add a clkdev lookup structure
+ * @hw: struct clk_hw to associate with all clk_lookups
+ * @con_id: connection ID string on device
+ * @dev_fmt: format string describing device name
+ *
+ * Returns a clk_lookup structure, which can be later unregistered and
+ * freed.
+ */
+struct clk_lookup *clkdev_hw_create(struct clk_hw *hw, const char *con_id,
+   const char *dev_fmt, ...)
+{
+   struct clk_lookup *cl;
+   va_list ap;
+
+   va_start(ap, dev_fmt);
+   cl = vclkdev_create(hw, con_id, dev_fmt, ap);
+   va_end(ap);
+
+   return cl;
+}
+EXPORT_SYMBOL_GPL(clkdev_hw_create);
+
 int clk_add_alias(const char *alias, const char *alias_dev_name,
const char *con_id, struct device *dev)
 {
@@ -402,3 +439,30 @@ int clk_register_clkdev(struct clk *clk, const char 
*con_id,
return cl ? 0 : -ENOMEM;
 }
 EXPORT_SYMBOL(clk_register_clkdev);
+
+/**
+ * clk_hw_register_clkdev - register one clock lookup for a struct clk_hw
+ * @hw: struct clk_hw to associate with all clk_lookups
+ * @con_id: connection ID string on device
+ * @dev_id: format string describing device name
+ *
+ * con_id or dev_id may be NULL as a wildcard, just as in the rest of
+ * clkdev.
+ */
+int clk_hw_register_clkdev(struct clk_hw *hw, const char *con_id,
+   const char *dev_id)
+{
+   struct clk_lookup *cl;
+
+   /*
+* Since dev_id can be NULL, and NULL is handled specially, we must
+* pass it as either a NULL format string, or with "%s".
+*/
+   if (dev_id)
+   cl = __clk_register_clkdev(hw, con_id, "%s", dev_id);
+   else
+   cl = __clk_register_clkdev(hw, con_id, NULL);
+
+   return cl ? 0 : -ENOMEM;
+}
+EXPORT_SYMBOL(clk_hw_register_clkdev);
diff --git a/include/linux/clkdev.h b/include/linux/clkdev.h
index e6f8eb1d585f..2eabc862abdb 100644
--- a/include/linux/clkdev.h
+++ b/include/linux/clkdev.h
@@ -15,6 +15,7 @@
 #include 
 
 struct clk;
+struct clk_hw;
 struct device;
 
 struct clk_lookup {
@@ -34,17 +35,22 @@ struct clk_lookup {
 
 struct clk_lookup *clkdev_alloc(struct clk *clk, const char *con_id,
const char *dev_fmt, ...) __printf(3, 4);
+struct clk_lookup *clkdev_hw_alloc(struct clk_hw *hw, const char *con_id,
+   const char *dev_fmt, ...) __printf(3, 4);
 
 void clkdev_add(struct clk_lookup *cl);
 void clkdev_drop(struct clk_lookup *cl);
 
 struct clk_lookup *clkdev_create(struct clk *clk, const char *con_id,
const char *dev_fmt, ...) __printf(3, 4);
+struct clk_lookup *clkdev_hw_create(struct clk_hw *hw, const char *con_id,
+   const char *dev_fmt, ...) __printf(3, 4);
 
 void clkdev_add_table(struct clk_lookup *, size_t);
 int clk_add_alias(const char *, const char *, const char *, struct device *);
 
 int clk_register_clkdev(struct clk *, const char *, const char *);
+int clk_hw_register_clkdev(struct clk_hw *, const char *, const char *);
 
 #ifdef CONFIG_COMMON_CLK
 int __clk_get(struct clk *clk);
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v2 02/13] clk: Add {devm_}clk_hw_{register,unregister}() APIs

[Stephen Boyd]

We've largely split the clk consumer and provider APIs along
struct clk and struct clk_hw, but clk_register() still returns a
struct clk pointer for each struct clk_hw that's registered.
Eventually we'd like to only allocate struct clks when there's a
user, because struct clk is per-user now, so clk_register() needs
to change.

Let's add new APIs to register struct clk_hws, but this time
we'll hide the struct clk from the caller by returning an int
error code. Also add an unregistration API that takes the clk_hw
structure that was passed to the registration API. This way
provider drivers never have to deal with a struct clk pointer
unless they're using the clk consumer APIs.

Signed-off-by: Stephen Boyd 
---
 Documentation/driver-model/devres.txt |  1 +
 drivers/clk/clk.c | 86 +++
 include/linux/clk-provider.h  |  6 +++
 3 files changed, 93 insertions(+)

diff --git a/Documentation/driver-model/devres.txt 
b/Documentation/driver-model/devres.txt
index 73b98dfbcea4..108d45553e1b 100644
--- a/Documentation/driver-model/devres.txt
+++ b/Documentation/driver-model/devres.txt
@@ -236,6 +236,7 @@ certainly invest a bit more effort into libata core layer).
 CLOCK
   devm_clk_get()
   devm_clk_put()
+  devm_clk_hw_register()
 
 DMA
   dmam_alloc_coherent()
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index fb74dc1f7520..0ef919666827 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2536,6 +2536,22 @@ fail_out:
 }
 EXPORT_SYMBOL_GPL(clk_register);
 
+/**
+ * clk_hw_register - register a clk_hw and return an error code
+ * @dev: device that is registering this clock
+ * @hw: link to hardware-specific clock data
+ *
+ * clk_hw_register is the primary interface for populating the clock tree with
+ * new clock nodes. It returns an integer equal to zero indicating success or
+ * less than zero indicating failure. Drivers must test for an error code after
+ * calling clk_hw_register().
+ */
+int clk_hw_register(struct device *dev, struct clk_hw *hw)
+{
+   return PTR_ERR_OR_ZERO(clk_register(dev, hw));
+}
+EXPORT_SYMBOL_GPL(clk_hw_register);
+
 /* Free memory allocated for a clock. */
 static void __clk_release(struct kref *ref)
 {
@@ -2637,11 +2653,26 @@ unlock:
 }
 EXPORT_SYMBOL_GPL(clk_unregister);
 
+/**
+ * clk_hw_unregister - unregister a currently registered clk_hw
+ * @hw: hardware-specific clock data to unregister
+ */
+void clk_hw_unregister(struct clk_hw *hw)
+{
+   clk_unregister(hw->clk);
+}
+EXPORT_SYMBOL_GPL(clk_hw_unregister);
+
 static void devm_clk_release(struct device *dev, void *res)
 {
clk_unregister(*(struct clk **)res);
 }
 
+static void devm_clk_hw_release(struct device *dev, void *res)
+{
+   clk_hw_unregister(*(struct clk_hw **)res);
+}
+
 /**
  * devm_clk_register - resource managed clk_register()
  * @dev: device that is registering this clock
@@ -2672,6 +2703,36 @@ struct clk *devm_clk_register(struct device *dev, struct 
clk_hw *hw)
 }
 EXPORT_SYMBOL_GPL(devm_clk_register);
 
+/**
+ * devm_clk_hw_register - resource managed clk_hw_register()
+ * @dev: device that is registering this clock
+ * @hw: link to hardware-specific clock data
+ *
+ * Managed clk_hw_register(). Clocks returned from this function are
+ * automatically clk_hw_unregister()ed on driver detach. See clk_hw_register()
+ * for more information.
+ */
+int devm_clk_hw_register(struct device *dev, struct clk_hw *hw)
+{
+   struct clk_hw **hwp;
+   int ret;
+
+   hwp = devres_alloc(devm_clk_hw_release, sizeof(*hwp), GFP_KERNEL);
+   if (!hwp)
+   return -ENOMEM;
+
+   ret = clk_hw_register(dev, hw);
+   if (!ret) {
+   *hwp = hw;
+   devres_add(dev, hwp);
+   } else {
+   devres_free(hwp);
+   }
+
+   return ret;
+}
+EXPORT_SYMBOL_GPL(devm_clk_hw_register);
+
 static int devm_clk_match(struct device *dev, void *res, void *data)
 {
struct clk *c = res;
@@ -2680,6 +2741,15 @@ static int devm_clk_match(struct device *dev, void *res, 
void *data)
return c == data;
 }
 
+static int devm_clk_hw_match(struct device *dev, void *res, void *data)
+{
+   struct clk_hw *hw = res;
+
+   if (WARN_ON(!hw))
+   return 0;
+   return hw == data;
+}
+
 /**
  * devm_clk_unregister - resource managed clk_unregister()
  * @clk: clock to unregister
@@ -2694,6 +2764,22 @@ void devm_clk_unregister(struct device *dev, struct clk 
*clk)
 }
 EXPORT_SYMBOL_GPL(devm_clk_unregister);
 
+/**
+ * devm_clk_hw_unregister - resource managed clk_hw_unregister()
+ * @dev: device that is unregistering the hardware-specific clock data
+ * @hw: link to hardware-specific clock data
+ *
+ * Unregister a clk_hw registered with devm_clk_hw_register(). Normally
+ * this function will not need to be called and the resource management
+ * code will ensure that the resource is freed.
+ */
+void devm_clk_hw_unregister(struct device *dev, struct clk_hw *hw)
+{
+  

[PATCH v2 00/13] clk_hw based clkdev/DT providers

[Stephen Boyd]

We've mostly split the clk API between consumers and providers
along struct clk and struct clk_hw, but the registration and
clkdev/DT code is still struct clk focused. This series
lays the foundation for changing that design by making
clk_register() return an int instead of a struct clk and
clkdev/DT lookups take a struct clk_hw instead of a struct clk.

After this series is applied, we can avoid using struct clk
in provider drivers entirely, unless we want to use the consumer
APIs. There are quite a few registration callers, so this change
will require converting all of them. I plan to do that gradually
over the coming weeks, but I've included a change to qcom
platforms because I could easily test it and show how things
will work.

The first patch is a cleanup to the clkdev APIs that I found
while doing this work. I'd like to take that into clk-next
with Russell's ack.

Cc: Russell King 

Stephen Boyd (13):
  clkdev: Remove clk_register_clkdevs()
  clk: Add {devm_}clk_hw_{register,unregister}() APIs
  clk: Add clk_hw OF clk providers
  clkdev: Add clk_hw based registration APIs
  clk: divider: Add hw based registration APIs
  clk: gate: Add hw based registration APIs
  clk: mux: Add hw based registration APIs
  clk: fixed-factor: Add hw based registration APIs
  clk: fractional-divider: Add hw based registration APIs
  clk: composite: Add hw based registration APIs
  clk: gpio: Add hw based registration APIs
  clk: fixed-rate: Add hw based registration APIs
  clk: qcom: Migrate to clk_hw based registration and OF APIs

 Documentation/driver-model/devres.txt |   1 +
 drivers/clk/clk-composite.c   |  45 ++---
 drivers/clk/clk-divider.c |  91 --
 drivers/clk/clk-fixed-factor.c|  42 +++--
 drivers/clk/clk-fixed-rate.c  |  44 +++--
 drivers/clk/clk-fractional-divider.c  |  40 +++-
 drivers/clk/clk-gate.c|  43 +++--
 drivers/clk/clk-gpio.c|  52 ---
 drivers/clk/clk-mux.c |  57 ++--
 drivers/clk/clk.c | 171 +-
 drivers/clk/clkdev.c  |  75 +++
 drivers/clk/qcom/clk-regmap.c |   5 +-
 drivers/clk/qcom/clk-regmap.h |   3 +-
 drivers/clk/qcom/common.c |  58 ++--
 drivers/clk/qcom/gcc-msm8996.c|   9 +-
 drivers/clk/qcom/mmcc-msm8996.c   |   9 +-
 include/linux/clk-provider.h  |  93 ++
 include/linux/clkdev.h|   7 +-
 18 files changed, 716 insertions(+), 129 deletions(-)

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v2 10/13] clk: composite: Add hw based registration APIs

[Stephen Boyd]

Add registration APIs in the clk composite code to return struct
clk_hw pointers instead of struct clk pointers. This way we hide
the struct clk pointer from providers unless they need to use
consumer facing APIs.

Signed-off-by: Stephen Boyd 
---
 drivers/clk/clk-composite.c  | 45 
 include/linux/clk-provider.h |  7 +++
 2 files changed, 40 insertions(+), 12 deletions(-)

diff --git a/drivers/clk/clk-composite.c b/drivers/clk/clk-composite.c
index 1f903e1f86a2..463fadd5a68f 100644
--- a/drivers/clk/clk-composite.c
+++ b/drivers/clk/clk-composite.c
@@ -184,17 +184,18 @@ static void clk_composite_disable(struct clk_hw *hw)
gate_ops->disable(gate_hw);
 }
 
-struct clk *clk_register_composite(struct device *dev, const char *name,
+struct clk_hw *clk_hw_register_composite(struct device *dev, const char *name,
const char * const *parent_names, int num_parents,
struct clk_hw *mux_hw, const struct clk_ops *mux_ops,
struct clk_hw *rate_hw, const struct clk_ops *rate_ops,
struct clk_hw *gate_hw, const struct clk_ops *gate_ops,
unsigned long flags)
 {
-   struct clk *clk;
+   struct clk_hw *hw;
struct clk_init_data init;
struct clk_composite *composite;
struct clk_ops *clk_composite_ops;
+   int ret;
 
composite = kzalloc(sizeof(*composite), GFP_KERNEL);
if (!composite)
@@ -204,12 +205,13 @@ struct clk *clk_register_composite(struct device *dev, 
const char *name,
init.flags = flags | CLK_IS_BASIC;
init.parent_names = parent_names;
init.num_parents = num_parents;
+   hw = >hw;
 
clk_composite_ops = >ops;
 
if (mux_hw && mux_ops) {
if (!mux_ops->get_parent) {
-   clk = ERR_PTR(-EINVAL);
+   hw = ERR_PTR(-EINVAL);
goto err;
}
 
@@ -224,7 +226,7 @@ struct clk *clk_register_composite(struct device *dev, 
const char *name,
 
if (rate_hw && rate_ops) {
if (!rate_ops->recalc_rate) {
-   clk = ERR_PTR(-EINVAL);
+   hw = ERR_PTR(-EINVAL);
goto err;
}
clk_composite_ops->recalc_rate = clk_composite_recalc_rate;
@@ -253,7 +255,7 @@ struct clk *clk_register_composite(struct device *dev, 
const char *name,
if (gate_hw && gate_ops) {
if (!gate_ops->is_enabled || !gate_ops->enable ||
!gate_ops->disable) {
-   clk = ERR_PTR(-EINVAL);
+   hw = ERR_PTR(-EINVAL);
goto err;
}
 
@@ -267,22 +269,41 @@ struct clk *clk_register_composite(struct device *dev, 
const char *name,
init.ops = clk_composite_ops;
composite->hw.init = 
 
-   clk = clk_register(dev, >hw);
-   if (IS_ERR(clk))
+   ret = clk_hw_register(dev, hw);
+   if (ret) {
+   hw = ERR_PTR(ret);
goto err;
+   }
 
if (composite->mux_hw)
-   composite->mux_hw->clk = clk;
+   composite->mux_hw->clk = hw->clk;
 
if (composite->rate_hw)
-   composite->rate_hw->clk = clk;
+   composite->rate_hw->clk = hw->clk;
 
if (composite->gate_hw)
-   composite->gate_hw->clk = clk;
+   composite->gate_hw->clk = hw->clk;
 
-   return clk;
+   return hw;
 
 err:
kfree(composite);
-   return clk;
+   return hw;
+}
+
+struct clk *clk_register_composite(struct device *dev, const char *name,
+   const char * const *parent_names, int num_parents,
+   struct clk_hw *mux_hw, const struct clk_ops *mux_ops,
+   struct clk_hw *rate_hw, const struct clk_ops *rate_ops,
+   struct clk_hw *gate_hw, const struct clk_ops *gate_ops,
+   unsigned long flags)
+{
+   struct clk_hw *hw;
+
+   hw = clk_hw_register_composite(dev, name, parent_names, num_parents,
+   mux_hw, mux_ops, rate_hw, rate_ops, gate_hw, gate_ops,
+   flags);
+   if (IS_ERR(hw))
+   return ERR_CAST(hw);
+   return hw->clk;
 }
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index bcbaf6c95d52..456c3ced1ac9 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -638,6 +638,13 @@ struct clk *clk_register_composite(struct device *dev, 
const char *name,
struct clk_hw *rate_hw, const struct clk_ops *rate_ops,
struct clk_hw *gate_hw, const struct clk_ops *gate_ops,
unsigned long flags);
+struct clk_hw *clk_hw_register_composite(struct device *dev, const char *name,
+   const char * const *parent_names, 

[PATCH v2 04/13] clkdev: Add clk_hw based registration APIs

[Stephen Boyd]

Now that we have a clk registration API that doesn't return
struct clks, we need to have some way to hand out struct clks via
the clk_get() APIs that doesn't involve associating struct clk
pointers with a struct clk_lookup. Luckily, clkdev already
operates on struct clk_hw pointers, except for the registration
facing APIs where it converts struct clk pointers into struct
clk_hw pointers almost immediately.

Let's add clk_hw based registration APIs so that we can skip the
conversion step and provide a way for clk provider drivers to
operate exclusively on clk_hw structs. This way we clearly
split the API between consumers and providers.

Cc: Russell King 
Signed-off-by: Stephen Boyd 
---
 drivers/clk/clkdev.c   | 64 ++
 include/linux/clkdev.h |  6 +
 2 files changed, 70 insertions(+)

diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c
index ae8e40a82d34..89cc700fbc37 100644
--- a/drivers/clk/clkdev.c
+++ b/drivers/clk/clkdev.c
@@ -301,6 +301,20 @@ clkdev_alloc(struct clk *clk, const char *con_id, const 
char *dev_fmt, ...)
 }
 EXPORT_SYMBOL(clkdev_alloc);
 
+struct clk_lookup *
+clkdev_hw_alloc(struct clk_hw *hw, const char *con_id, const char *dev_fmt, 
...)
+{
+   struct clk_lookup *cl;
+   va_list ap;
+
+   va_start(ap, dev_fmt);
+   cl = vclkdev_alloc(hw, con_id, dev_fmt, ap);
+   va_end(ap);
+
+   return cl;
+}
+EXPORT_SYMBOL(clkdev_hw_alloc);
+
 /**
  * clkdev_create - allocate and add a clkdev lookup structure
  * @clk: struct clk to associate with all clk_lookups
@@ -324,6 +338,29 @@ struct clk_lookup *clkdev_create(struct clk *clk, const 
char *con_id,
 }
 EXPORT_SYMBOL_GPL(clkdev_create);
 
+/**
+ * clkdev_hw_create - allocate and add a clkdev lookup structure
+ * @hw: struct clk_hw to associate with all clk_lookups
+ * @con_id: connection ID string on device
+ * @dev_fmt: format string describing device name
+ *
+ * Returns a clk_lookup structure, which can be later unregistered and
+ * freed.
+ */
+struct clk_lookup *clkdev_hw_create(struct clk_hw *hw, const char *con_id,
+   const char *dev_fmt, ...)
+{
+   struct clk_lookup *cl;
+   va_list ap;
+
+   va_start(ap, dev_fmt);
+   cl = vclkdev_create(hw, con_id, dev_fmt, ap);
+   va_end(ap);
+
+   return cl;
+}
+EXPORT_SYMBOL_GPL(clkdev_hw_create);
+
 int clk_add_alias(const char *alias, const char *alias_dev_name,
const char *con_id, struct device *dev)
 {
@@ -402,3 +439,30 @@ int clk_register_clkdev(struct clk *clk, const char 
*con_id,
return cl ? 0 : -ENOMEM;
 }
 EXPORT_SYMBOL(clk_register_clkdev);
+
+/**
+ * clk_hw_register_clkdev - register one clock lookup for a struct clk_hw
+ * @hw: struct clk_hw to associate with all clk_lookups
+ * @con_id: connection ID string on device
+ * @dev_id: format string describing device name
+ *
+ * con_id or dev_id may be NULL as a wildcard, just as in the rest of
+ * clkdev.
+ */
+int clk_hw_register_clkdev(struct clk_hw *hw, const char *con_id,
+   const char *dev_id)
+{
+   struct clk_lookup *cl;
+
+   /*
+* Since dev_id can be NULL, and NULL is handled specially, we must
+* pass it as either a NULL format string, or with "%s".
+*/
+   if (dev_id)
+   cl = __clk_register_clkdev(hw, con_id, "%s", dev_id);
+   else
+   cl = __clk_register_clkdev(hw, con_id, NULL);
+
+   return cl ? 0 : -ENOMEM;
+}
+EXPORT_SYMBOL(clk_hw_register_clkdev);
diff --git a/include/linux/clkdev.h b/include/linux/clkdev.h
index e6f8eb1d585f..2eabc862abdb 100644
--- a/include/linux/clkdev.h
+++ b/include/linux/clkdev.h
@@ -15,6 +15,7 @@
 #include 
 
 struct clk;
+struct clk_hw;
 struct device;
 
 struct clk_lookup {
@@ -34,17 +35,22 @@ struct clk_lookup {
 
 struct clk_lookup *clkdev_alloc(struct clk *clk, const char *con_id,
const char *dev_fmt, ...) __printf(3, 4);
+struct clk_lookup *clkdev_hw_alloc(struct clk_hw *hw, const char *con_id,
+   const char *dev_fmt, ...) __printf(3, 4);
 
 void clkdev_add(struct clk_lookup *cl);
 void clkdev_drop(struct clk_lookup *cl);
 
 struct clk_lookup *clkdev_create(struct clk *clk, const char *con_id,
const char *dev_fmt, ...) __printf(3, 4);
+struct clk_lookup *clkdev_hw_create(struct clk_hw *hw, const char *con_id,
+   const char *dev_fmt, ...) __printf(3, 4);
 
 void clkdev_add_table(struct clk_lookup *, size_t);
 int clk_add_alias(const char *, const char *, const char *, struct device *);
 
 int clk_register_clkdev(struct clk *, const char *, const char *);
+int clk_hw_register_clkdev(struct clk_hw *, const char *, const char *);
 
 #ifdef CONFIG_COMMON_CLK
 int __clk_get(struct clk *clk);
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v2 03/13] clk: Add clk_hw OF clk providers

[Stephen Boyd]

Now that we have a clk registration API that doesn't return
struct clks, we need to have some way to hand out struct clks via
the clk_get() APIs that doesn't involve associating struct clk
pointers with an OF node. Currently we ask the OF provider to
give us a struct clk pointer for some clkspec, turn that struct
clk into a struct clk_hw and then allocate a new struct clk to
return to the caller.

Let's add a clk_hw based OF provider hook that returns a struct
clk_hw directly, so that we skip the intermediate step of
converting from struct clk to struct clk_hw. Eventually when
we've converted all OF clk providers to struct clk_hw based APIs
we can remove the struct clk based ones.

It should also be noted that we change the onecell provider to
have a flex array instead of a pointer for the array of clk_hw
pointers. This allows providers to allocate one structure of the
correct length in one step instead of two.

Signed-off-by: Stephen Boyd 
---
 drivers/clk/clk.c| 85 +---
 include/linux/clk-provider.h | 30 
 2 files changed, 111 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 0ef919666827..e813b2aabc87 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2941,6 +2941,7 @@ struct of_clk_provider {
 
struct device_node *node;
struct clk *(*get)(struct of_phandle_args *clkspec, void *data);
+   struct clk_hw *(*get_hw)(struct of_phandle_args *clkspec, void *data);
void *data;
 };
 
@@ -2957,6 +2958,12 @@ struct clk *of_clk_src_simple_get(struct of_phandle_args 
*clkspec,
 }
 EXPORT_SYMBOL_GPL(of_clk_src_simple_get);
 
+struct clk_hw *of_clk_hw_simple_get(struct of_phandle_args *clkspec, void 
*data)
+{
+   return data;
+}
+EXPORT_SYMBOL_GPL(of_clk_hw_simple_get);
+
 struct clk *of_clk_src_onecell_get(struct of_phandle_args *clkspec, void *data)
 {
struct clk_onecell_data *clk_data = data;
@@ -2971,6 +2978,21 @@ struct clk *of_clk_src_onecell_get(struct 
of_phandle_args *clkspec, void *data)
 }
 EXPORT_SYMBOL_GPL(of_clk_src_onecell_get);
 
+struct clk_hw *
+of_clk_hw_onecell_get(struct of_phandle_args *clkspec, void *data)
+{
+   struct clk_hw_onecell_data *hw_data = data;
+   unsigned int idx = clkspec->args[0];
+
+   if (idx >= hw_data->num) {
+   pr_err("%s: invalid index %u\n", __func__, idx);
+   return ERR_PTR(-EINVAL);
+   }
+
+   return hw_data->hws[idx];
+}
+EXPORT_SYMBOL_GPL(of_clk_hw_onecell_get);
+
 /**
  * of_clk_add_provider() - Register a clock provider for a node
  * @np: Device node pointer associated with clock provider
@@ -3007,6 +3029,41 @@ int of_clk_add_provider(struct device_node *np,
 EXPORT_SYMBOL_GPL(of_clk_add_provider);
 
 /**
+ * of_clk_add_hw_provider() - Register a clock provider for a node
+ * @np: Device node pointer associated with clock provider
+ * @get: callback for decoding clk_hw
+ * @data: context pointer for @get callback.
+ */
+int of_clk_add_hw_provider(struct device_node *np,
+  struct clk_hw *(*get)(struct of_phandle_args 
*clkspec,
+void *data),
+  void *data)
+{
+   struct of_clk_provider *cp;
+   int ret;
+
+   cp = kzalloc(sizeof(*cp), GFP_KERNEL);
+   if (!cp)
+   return -ENOMEM;
+
+   cp->node = of_node_get(np);
+   cp->data = data;
+   cp->get_hw = get;
+
+   mutex_lock(_clk_mutex);
+   list_add(>link, _clk_providers);
+   mutex_unlock(_clk_mutex);
+   pr_debug("Added clk_hw provider from %s\n", np->full_name);
+
+   ret = of_clk_set_defaults(np, true);
+   if (ret < 0)
+   of_clk_del_provider(np);
+
+   return ret;
+}
+EXPORT_SYMBOL_GPL(of_clk_add_hw_provider);
+
+/**
  * of_clk_del_provider() - Remove a previously registered clock provider
  * @np: Device node pointer associated with clock provider
  */
@@ -3027,11 +3084,32 @@ void of_clk_del_provider(struct device_node *np)
 }
 EXPORT_SYMBOL_GPL(of_clk_del_provider);
 
+static struct clk_hw *
+__of_clk_get_hw_from_provider(struct of_clk_provider *provider,
+ struct of_phandle_args *clkspec)
+{
+   struct clk *clk;
+   struct clk_hw *hw = ERR_PTR(-EPROBE_DEFER);
+
+   if (provider->get_hw) {
+   hw = provider->get_hw(clkspec, provider->data);
+   } else if (provider->get) {
+   clk = provider->get(clkspec, provider->data);
+   if (!IS_ERR(clk))
+   hw = __clk_get_hw(clk);
+   else
+   hw = ERR_CAST(clk);
+   }
+
+   return hw;
+}
+
 struct clk *__of_clk_get_from_provider(struct of_phandle_args *clkspec,
   const char *dev_id, const char *con_id)
 {
struct of_clk_provider *provider;
struct clk *clk = ERR_PTR(-EPROBE_DEFER);
+  

[PATCH v2 03/13] clk: Add clk_hw OF clk providers

[Stephen Boyd]

Now that we have a clk registration API that doesn't return
struct clks, we need to have some way to hand out struct clks via
the clk_get() APIs that doesn't involve associating struct clk
pointers with an OF node. Currently we ask the OF provider to
give us a struct clk pointer for some clkspec, turn that struct
clk into a struct clk_hw and then allocate a new struct clk to
return to the caller.

Let's add a clk_hw based OF provider hook that returns a struct
clk_hw directly, so that we skip the intermediate step of
converting from struct clk to struct clk_hw. Eventually when
we've converted all OF clk providers to struct clk_hw based APIs
we can remove the struct clk based ones.

It should also be noted that we change the onecell provider to
have a flex array instead of a pointer for the array of clk_hw
pointers. This allows providers to allocate one structure of the
correct length in one step instead of two.

Signed-off-by: Stephen Boyd 
---
 drivers/clk/clk.c| 85 +---
 include/linux/clk-provider.h | 30 
 2 files changed, 111 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 0ef919666827..e813b2aabc87 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2941,6 +2941,7 @@ struct of_clk_provider {
 
struct device_node *node;
struct clk *(*get)(struct of_phandle_args *clkspec, void *data);
+   struct clk_hw *(*get_hw)(struct of_phandle_args *clkspec, void *data);
void *data;
 };
 
@@ -2957,6 +2958,12 @@ struct clk *of_clk_src_simple_get(struct of_phandle_args 
*clkspec,
 }
 EXPORT_SYMBOL_GPL(of_clk_src_simple_get);
 
+struct clk_hw *of_clk_hw_simple_get(struct of_phandle_args *clkspec, void 
*data)
+{
+   return data;
+}
+EXPORT_SYMBOL_GPL(of_clk_hw_simple_get);
+
 struct clk *of_clk_src_onecell_get(struct of_phandle_args *clkspec, void *data)
 {
struct clk_onecell_data *clk_data = data;
@@ -2971,6 +2978,21 @@ struct clk *of_clk_src_onecell_get(struct 
of_phandle_args *clkspec, void *data)
 }
 EXPORT_SYMBOL_GPL(of_clk_src_onecell_get);
 
+struct clk_hw *
+of_clk_hw_onecell_get(struct of_phandle_args *clkspec, void *data)
+{
+   struct clk_hw_onecell_data *hw_data = data;
+   unsigned int idx = clkspec->args[0];
+
+   if (idx >= hw_data->num) {
+   pr_err("%s: invalid index %u\n", __func__, idx);
+   return ERR_PTR(-EINVAL);
+   }
+
+   return hw_data->hws[idx];
+}
+EXPORT_SYMBOL_GPL(of_clk_hw_onecell_get);
+
 /**
  * of_clk_add_provider() - Register a clock provider for a node
  * @np: Device node pointer associated with clock provider
@@ -3007,6 +3029,41 @@ int of_clk_add_provider(struct device_node *np,
 EXPORT_SYMBOL_GPL(of_clk_add_provider);
 
 /**
+ * of_clk_add_hw_provider() - Register a clock provider for a node
+ * @np: Device node pointer associated with clock provider
+ * @get: callback for decoding clk_hw
+ * @data: context pointer for @get callback.
+ */
+int of_clk_add_hw_provider(struct device_node *np,
+  struct clk_hw *(*get)(struct of_phandle_args 
*clkspec,
+void *data),
+  void *data)
+{
+   struct of_clk_provider *cp;
+   int ret;
+
+   cp = kzalloc(sizeof(*cp), GFP_KERNEL);
+   if (!cp)
+   return -ENOMEM;
+
+   cp->node = of_node_get(np);
+   cp->data = data;
+   cp->get_hw = get;
+
+   mutex_lock(_clk_mutex);
+   list_add(>link, _clk_providers);
+   mutex_unlock(_clk_mutex);
+   pr_debug("Added clk_hw provider from %s\n", np->full_name);
+
+   ret = of_clk_set_defaults(np, true);
+   if (ret < 0)
+   of_clk_del_provider(np);
+
+   return ret;
+}
+EXPORT_SYMBOL_GPL(of_clk_add_hw_provider);
+
+/**
  * of_clk_del_provider() - Remove a previously registered clock provider
  * @np: Device node pointer associated with clock provider
  */
@@ -3027,11 +3084,32 @@ void of_clk_del_provider(struct device_node *np)
 }
 EXPORT_SYMBOL_GPL(of_clk_del_provider);
 
+static struct clk_hw *
+__of_clk_get_hw_from_provider(struct of_clk_provider *provider,
+ struct of_phandle_args *clkspec)
+{
+   struct clk *clk;
+   struct clk_hw *hw = ERR_PTR(-EPROBE_DEFER);
+
+   if (provider->get_hw) {
+   hw = provider->get_hw(clkspec, provider->data);
+   } else if (provider->get) {
+   clk = provider->get(clkspec, provider->data);
+   if (!IS_ERR(clk))
+   hw = __clk_get_hw(clk);
+   else
+   hw = ERR_CAST(clk);
+   }
+
+   return hw;
+}
+
 struct clk *__of_clk_get_from_provider(struct of_phandle_args *clkspec,
   const char *dev_id, const char *con_id)
 {
struct of_clk_provider *provider;
struct clk *clk = ERR_PTR(-EPROBE_DEFER);
+   struct clk_hw *hw = 

[PATCH v2 13/13] clk: qcom: Migrate to clk_hw based registration and OF APIs

[Stephen Boyd]

Signed-off-by: Stephen Boyd 
---
 drivers/clk/qcom/clk-regmap.c   |  5 ++--
 drivers/clk/qcom/clk-regmap.h   |  3 +--
 drivers/clk/qcom/common.c   | 58 +++--
 drivers/clk/qcom/gcc-msm8996.c  |  9 +++
 drivers/clk/qcom/mmcc-msm8996.c |  9 +++
 5 files changed, 43 insertions(+), 41 deletions(-)

diff --git a/drivers/clk/qcom/clk-regmap.c b/drivers/clk/qcom/clk-regmap.c
index a58ba39a900c..1c856d330733 100644
--- a/drivers/clk/qcom/clk-regmap.c
+++ b/drivers/clk/qcom/clk-regmap.c
@@ -101,14 +101,13 @@ EXPORT_SYMBOL_GPL(clk_disable_regmap);
  * clk_regmap struct via this function so that the regmap is initialized
  * and so that the clock is registered with the common clock framework.
  */
-struct clk *devm_clk_register_regmap(struct device *dev,
-struct clk_regmap *rclk)
+int devm_clk_register_regmap(struct device *dev, struct clk_regmap *rclk)
 {
if (dev && dev_get_regmap(dev, NULL))
rclk->regmap = dev_get_regmap(dev, NULL);
else if (dev && dev->parent)
rclk->regmap = dev_get_regmap(dev->parent, NULL);
 
-   return devm_clk_register(dev, >hw);
+   return devm_clk_hw_register(dev, >hw);
 }
 EXPORT_SYMBOL_GPL(devm_clk_register_regmap);
diff --git a/drivers/clk/qcom/clk-regmap.h b/drivers/clk/qcom/clk-regmap.h
index 491a63d537df..90d95cd11ec6 100644
--- a/drivers/clk/qcom/clk-regmap.h
+++ b/drivers/clk/qcom/clk-regmap.h
@@ -39,7 +39,6 @@ struct clk_regmap {
 int clk_is_enabled_regmap(struct clk_hw *hw);
 int clk_enable_regmap(struct clk_hw *hw);
 void clk_disable_regmap(struct clk_hw *hw);
-struct clk *
-devm_clk_register_regmap(struct device *dev, struct clk_regmap *rclk);
+int devm_clk_register_regmap(struct device *dev, struct clk_regmap *rclk);
 
 #endif
diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c
index f7c226ab4307..fffcbaf0fba7 100644
--- a/drivers/clk/qcom/common.c
+++ b/drivers/clk/qcom/common.c
@@ -27,8 +27,8 @@
 
 struct qcom_cc {
struct qcom_reset_controller reset;
-   struct clk_onecell_data data;
-   struct clk *clks[];
+   struct clk_regmap **rclks;
+   size_t num_rclks;
 };
 
 const
@@ -102,8 +102,8 @@ static int _qcom_cc_register_board_clk(struct device *dev, 
const char *path,
struct device_node *clocks_node;
struct clk_fixed_factor *factor;
struct clk_fixed_rate *fixed;
-   struct clk *clk;
struct clk_init_data init_data = { };
+   int ret;
 
clocks_node = of_find_node_by_path("/clocks");
if (clocks_node)
@@ -121,9 +121,9 @@ static int _qcom_cc_register_board_clk(struct device *dev, 
const char *path,
init_data.name = path;
init_data.ops = _fixed_rate_ops;
 
-   clk = devm_clk_register(dev, >hw);
-   if (IS_ERR(clk))
-   return PTR_ERR(clk);
+   ret = devm_clk_hw_register(dev, >hw);
+   if (ret)
+   return ret;
}
of_node_put(node);
 
@@ -141,9 +141,9 @@ static int _qcom_cc_register_board_clk(struct device *dev, 
const char *path,
init_data.flags = 0;
init_data.ops = _fixed_factor_ops;
 
-   clk = devm_clk_register(dev, >hw);
-   if (IS_ERR(clk))
-   return PTR_ERR(clk);
+   ret = devm_clk_hw_register(dev, >hw);
+   if (ret)
+   return ret;
}
 
return 0;
@@ -174,42 +174,48 @@ int qcom_cc_register_sleep_clk(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(qcom_cc_register_sleep_clk);
 
+static struct clk_hw *qcom_cc_clk_hw_get(struct of_phandle_args *clkspec,
+void *data)
+{
+   struct qcom_cc *cc = data;
+   unsigned int idx = clkspec->args[0];
+
+   if (idx >= cc->num_rclks) {
+   pr_err("%s: invalid index %u\n", __func__, idx);
+   return ERR_PTR(-EINVAL);
+   }
+
+   return cc->rclks[idx] ? >rclks[idx]->hw : ERR_PTR(-ENOENT);
+}
+
 int qcom_cc_really_probe(struct platform_device *pdev,
 const struct qcom_cc_desc *desc, struct regmap *regmap)
 {
int i, ret;
struct device *dev = >dev;
-   struct clk *clk;
-   struct clk_onecell_data *data;
-   struct clk **clks;
struct qcom_reset_controller *reset;
struct qcom_cc *cc;
struct gdsc_desc *scd;
size_t num_clks = desc->num_clks;
struct clk_regmap **rclks = desc->clks;
 
-   cc = devm_kzalloc(dev, sizeof(*cc) + sizeof(*clks) * num_clks,
- GFP_KERNEL);
+   cc = devm_kzalloc(dev, sizeof(*cc), GFP_KERNEL);
if (!cc)
return -ENOMEM;
 
-   clks = cc->clks;
-   data = >data;
-   data->clks = clks;
-   data->clk_num = num_clks;
+   cc->rclks = rclks;
+   cc->num_rclks = num_clks;
 

[PATCH v2 13/13] clk: qcom: Migrate to clk_hw based registration and OF APIs

[Stephen Boyd]

Signed-off-by: Stephen Boyd 
---
 drivers/clk/qcom/clk-regmap.c   |  5 ++--
 drivers/clk/qcom/clk-regmap.h   |  3 +--
 drivers/clk/qcom/common.c   | 58 +++--
 drivers/clk/qcom/gcc-msm8996.c  |  9 +++
 drivers/clk/qcom/mmcc-msm8996.c |  9 +++
 5 files changed, 43 insertions(+), 41 deletions(-)

diff --git a/drivers/clk/qcom/clk-regmap.c b/drivers/clk/qcom/clk-regmap.c
index a58ba39a900c..1c856d330733 100644
--- a/drivers/clk/qcom/clk-regmap.c
+++ b/drivers/clk/qcom/clk-regmap.c
@@ -101,14 +101,13 @@ EXPORT_SYMBOL_GPL(clk_disable_regmap);
  * clk_regmap struct via this function so that the regmap is initialized
  * and so that the clock is registered with the common clock framework.
  */
-struct clk *devm_clk_register_regmap(struct device *dev,
-struct clk_regmap *rclk)
+int devm_clk_register_regmap(struct device *dev, struct clk_regmap *rclk)
 {
if (dev && dev_get_regmap(dev, NULL))
rclk->regmap = dev_get_regmap(dev, NULL);
else if (dev && dev->parent)
rclk->regmap = dev_get_regmap(dev->parent, NULL);
 
-   return devm_clk_register(dev, >hw);
+   return devm_clk_hw_register(dev, >hw);
 }
 EXPORT_SYMBOL_GPL(devm_clk_register_regmap);
diff --git a/drivers/clk/qcom/clk-regmap.h b/drivers/clk/qcom/clk-regmap.h
index 491a63d537df..90d95cd11ec6 100644
--- a/drivers/clk/qcom/clk-regmap.h
+++ b/drivers/clk/qcom/clk-regmap.h
@@ -39,7 +39,6 @@ struct clk_regmap {
 int clk_is_enabled_regmap(struct clk_hw *hw);
 int clk_enable_regmap(struct clk_hw *hw);
 void clk_disable_regmap(struct clk_hw *hw);
-struct clk *
-devm_clk_register_regmap(struct device *dev, struct clk_regmap *rclk);
+int devm_clk_register_regmap(struct device *dev, struct clk_regmap *rclk);
 
 #endif
diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c
index f7c226ab4307..fffcbaf0fba7 100644
--- a/drivers/clk/qcom/common.c
+++ b/drivers/clk/qcom/common.c
@@ -27,8 +27,8 @@
 
 struct qcom_cc {
struct qcom_reset_controller reset;
-   struct clk_onecell_data data;
-   struct clk *clks[];
+   struct clk_regmap **rclks;
+   size_t num_rclks;
 };
 
 const
@@ -102,8 +102,8 @@ static int _qcom_cc_register_board_clk(struct device *dev, 
const char *path,
struct device_node *clocks_node;
struct clk_fixed_factor *factor;
struct clk_fixed_rate *fixed;
-   struct clk *clk;
struct clk_init_data init_data = { };
+   int ret;
 
clocks_node = of_find_node_by_path("/clocks");
if (clocks_node)
@@ -121,9 +121,9 @@ static int _qcom_cc_register_board_clk(struct device *dev, 
const char *path,
init_data.name = path;
init_data.ops = _fixed_rate_ops;
 
-   clk = devm_clk_register(dev, >hw);
-   if (IS_ERR(clk))
-   return PTR_ERR(clk);
+   ret = devm_clk_hw_register(dev, >hw);
+   if (ret)
+   return ret;
}
of_node_put(node);
 
@@ -141,9 +141,9 @@ static int _qcom_cc_register_board_clk(struct device *dev, 
const char *path,
init_data.flags = 0;
init_data.ops = _fixed_factor_ops;
 
-   clk = devm_clk_register(dev, >hw);
-   if (IS_ERR(clk))
-   return PTR_ERR(clk);
+   ret = devm_clk_hw_register(dev, >hw);
+   if (ret)
+   return ret;
}
 
return 0;
@@ -174,42 +174,48 @@ int qcom_cc_register_sleep_clk(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(qcom_cc_register_sleep_clk);
 
+static struct clk_hw *qcom_cc_clk_hw_get(struct of_phandle_args *clkspec,
+void *data)
+{
+   struct qcom_cc *cc = data;
+   unsigned int idx = clkspec->args[0];
+
+   if (idx >= cc->num_rclks) {
+   pr_err("%s: invalid index %u\n", __func__, idx);
+   return ERR_PTR(-EINVAL);
+   }
+
+   return cc->rclks[idx] ? >rclks[idx]->hw : ERR_PTR(-ENOENT);
+}
+
 int qcom_cc_really_probe(struct platform_device *pdev,
 const struct qcom_cc_desc *desc, struct regmap *regmap)
 {
int i, ret;
struct device *dev = >dev;
-   struct clk *clk;
-   struct clk_onecell_data *data;
-   struct clk **clks;
struct qcom_reset_controller *reset;
struct qcom_cc *cc;
struct gdsc_desc *scd;
size_t num_clks = desc->num_clks;
struct clk_regmap **rclks = desc->clks;
 
-   cc = devm_kzalloc(dev, sizeof(*cc) + sizeof(*clks) * num_clks,
- GFP_KERNEL);
+   cc = devm_kzalloc(dev, sizeof(*cc), GFP_KERNEL);
if (!cc)
return -ENOMEM;
 
-   clks = cc->clks;
-   data = >data;
-   data->clks = clks;
-   data->clk_num = num_clks;
+   cc->rclks = rclks;
+   cc->num_rclks = num_clks;
 
for (i = 0; i < 

[PATCH v2 06/13] clk: gate: Add hw based registration APIs

[Stephen Boyd]

Add registration APIs in the clk gate code to return struct
clk_hw pointers instead of struct clk pointers. This way we hide
the struct clk pointer from providers unless they need to use
consumer facing APIs.

Signed-off-by: Stephen Boyd 
---
 drivers/clk/clk-gate.c   | 43 ---
 include/linux/clk-provider.h |  5 +
 2 files changed, 41 insertions(+), 7 deletions(-)

diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c
index d0d8ec8e1f1b..4e691e35483a 100644
--- a/drivers/clk/clk-gate.c
+++ b/drivers/clk/clk-gate.c
@@ -110,7 +110,7 @@ const struct clk_ops clk_gate_ops = {
 EXPORT_SYMBOL_GPL(clk_gate_ops);
 
 /**
- * clk_register_gate - register a gate clock with the clock framework
+ * clk_hw_register_gate - register a gate clock with the clock framework
  * @dev: device that is registering this clock
  * @name: name of this clock
  * @parent_name: name of this clock's parent
@@ -120,14 +120,15 @@ EXPORT_SYMBOL_GPL(clk_gate_ops);
  * @clk_gate_flags: gate-specific flags for this clock
  * @lock: shared register lock for this clock
  */
-struct clk *clk_register_gate(struct device *dev, const char *name,
+struct clk_hw *clk_hw_register_gate(struct device *dev, const char *name,
const char *parent_name, unsigned long flags,
void __iomem *reg, u8 bit_idx,
u8 clk_gate_flags, spinlock_t *lock)
 {
struct clk_gate *gate;
-   struct clk *clk;
+   struct clk_hw *hw;
struct clk_init_data init;
+   int ret;
 
if (clk_gate_flags & CLK_GATE_HIWORD_MASK) {
if (bit_idx > 15) {
@@ -154,12 +155,29 @@ struct clk *clk_register_gate(struct device *dev, const 
char *name,
gate->lock = lock;
gate->hw.init = 
 
-   clk = clk_register(dev, >hw);
-
-   if (IS_ERR(clk))
+   hw = >hw;
+   ret = clk_hw_register(dev, hw);
+   if (ret) {
kfree(gate);
+   hw = ERR_PTR(ret);
+   }
 
-   return clk;
+   return hw;
+}
+EXPORT_SYMBOL_GPL(clk_hw_register_gate);
+
+struct clk *clk_register_gate(struct device *dev, const char *name,
+   const char *parent_name, unsigned long flags,
+   void __iomem *reg, u8 bit_idx,
+   u8 clk_gate_flags, spinlock_t *lock)
+{
+   struct clk_hw *hw;
+
+   hw = clk_hw_register_gate(dev, name, parent_name, flags, reg,
+ bit_idx, clk_gate_flags, lock);
+   if (IS_ERR(hw))
+   return ERR_CAST(hw);
+   return hw->clk;
 }
 EXPORT_SYMBOL_GPL(clk_register_gate);
 
@@ -178,3 +196,14 @@ void clk_unregister_gate(struct clk *clk)
kfree(gate);
 }
 EXPORT_SYMBOL_GPL(clk_unregister_gate);
+
+void clk_hw_unregister_gate(struct clk_hw *hw)
+{
+   struct clk_gate *gate;
+
+   gate = to_clk_gate(hw);
+
+   clk_hw_unregister(hw);
+   kfree(gate);
+}
+EXPORT_SYMBOL_GPL(clk_hw_unregister_gate);
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 8885d0350596..bf12050aadd5 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -326,7 +326,12 @@ struct clk *clk_register_gate(struct device *dev, const 
char *name,
const char *parent_name, unsigned long flags,
void __iomem *reg, u8 bit_idx,
u8 clk_gate_flags, spinlock_t *lock);
+struct clk_hw *clk_hw_register_gate(struct device *dev, const char *name,
+   const char *parent_name, unsigned long flags,
+   void __iomem *reg, u8 bit_idx,
+   u8 clk_gate_flags, spinlock_t *lock);
 void clk_unregister_gate(struct clk *clk);
+void clk_hw_unregister_gate(struct clk_hw *hw);
 
 struct clk_div_table {
unsigned intval;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v2 06/13] clk: gate: Add hw based registration APIs

[Stephen Boyd]

Add registration APIs in the clk gate code to return struct
clk_hw pointers instead of struct clk pointers. This way we hide
the struct clk pointer from providers unless they need to use
consumer facing APIs.

Signed-off-by: Stephen Boyd 
---
 drivers/clk/clk-gate.c   | 43 ---
 include/linux/clk-provider.h |  5 +
 2 files changed, 41 insertions(+), 7 deletions(-)

diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c
index d0d8ec8e1f1b..4e691e35483a 100644
--- a/drivers/clk/clk-gate.c
+++ b/drivers/clk/clk-gate.c
@@ -110,7 +110,7 @@ const struct clk_ops clk_gate_ops = {
 EXPORT_SYMBOL_GPL(clk_gate_ops);
 
 /**
- * clk_register_gate - register a gate clock with the clock framework
+ * clk_hw_register_gate - register a gate clock with the clock framework
  * @dev: device that is registering this clock
  * @name: name of this clock
  * @parent_name: name of this clock's parent
@@ -120,14 +120,15 @@ EXPORT_SYMBOL_GPL(clk_gate_ops);
  * @clk_gate_flags: gate-specific flags for this clock
  * @lock: shared register lock for this clock
  */
-struct clk *clk_register_gate(struct device *dev, const char *name,
+struct clk_hw *clk_hw_register_gate(struct device *dev, const char *name,
const char *parent_name, unsigned long flags,
void __iomem *reg, u8 bit_idx,
u8 clk_gate_flags, spinlock_t *lock)
 {
struct clk_gate *gate;
-   struct clk *clk;
+   struct clk_hw *hw;
struct clk_init_data init;
+   int ret;
 
if (clk_gate_flags & CLK_GATE_HIWORD_MASK) {
if (bit_idx > 15) {
@@ -154,12 +155,29 @@ struct clk *clk_register_gate(struct device *dev, const 
char *name,
gate->lock = lock;
gate->hw.init = 
 
-   clk = clk_register(dev, >hw);
-
-   if (IS_ERR(clk))
+   hw = >hw;
+   ret = clk_hw_register(dev, hw);
+   if (ret) {
kfree(gate);
+   hw = ERR_PTR(ret);
+   }
 
-   return clk;
+   return hw;
+}
+EXPORT_SYMBOL_GPL(clk_hw_register_gate);
+
+struct clk *clk_register_gate(struct device *dev, const char *name,
+   const char *parent_name, unsigned long flags,
+   void __iomem *reg, u8 bit_idx,
+   u8 clk_gate_flags, spinlock_t *lock)
+{
+   struct clk_hw *hw;
+
+   hw = clk_hw_register_gate(dev, name, parent_name, flags, reg,
+ bit_idx, clk_gate_flags, lock);
+   if (IS_ERR(hw))
+   return ERR_CAST(hw);
+   return hw->clk;
 }
 EXPORT_SYMBOL_GPL(clk_register_gate);
 
@@ -178,3 +196,14 @@ void clk_unregister_gate(struct clk *clk)
kfree(gate);
 }
 EXPORT_SYMBOL_GPL(clk_unregister_gate);
+
+void clk_hw_unregister_gate(struct clk_hw *hw)
+{
+   struct clk_gate *gate;
+
+   gate = to_clk_gate(hw);
+
+   clk_hw_unregister(hw);
+   kfree(gate);
+}
+EXPORT_SYMBOL_GPL(clk_hw_unregister_gate);
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 8885d0350596..bf12050aadd5 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -326,7 +326,12 @@ struct clk *clk_register_gate(struct device *dev, const 
char *name,
const char *parent_name, unsigned long flags,
void __iomem *reg, u8 bit_idx,
u8 clk_gate_flags, spinlock_t *lock);
+struct clk_hw *clk_hw_register_gate(struct device *dev, const char *name,
+   const char *parent_name, unsigned long flags,
+   void __iomem *reg, u8 bit_idx,
+   u8 clk_gate_flags, spinlock_t *lock);
 void clk_unregister_gate(struct clk *clk);
+void clk_hw_unregister_gate(struct clk_hw *hw);
 
 struct clk_div_table {
unsigned intval;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH] fs: fix over-zealous use of "const"

[Andy Shevchenko]

On Thu, Apr 21, 2016 at 10:53 PM, Kees Cook  wrote:
> When I was fixing up const recommendations from checkpatch.pl, I went
> overboard. This fixes the warning (during a W=1 build):
>
> include/linux/fs.h:2627:74: warning: type qualifiers ignored on function 
> return type [-Wignored-qualifiers]
> static inline const char * const kernel_read_file_id_str(enum 
> kernel_read_file_id id)
>
> Reported-by: Andy Shevchenko 
> Signed-off-by: Kees Cook 

Will test next week if someone don't do that before.
Thanks.

> ---
> This is for linux-security next
> ---
>  include/linux/fs.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 518716b4834e..82f9a2db3b1a 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2610,7 +2610,7 @@ static const char * const kernel_read_file_str[] = {
> __kernel_read_file_id(__fid_stringify)
>  };
>
> -static inline const char * const kernel_read_file_id_str(enum 
> kernel_read_file_id id)
> +static inline const char *kernel_read_file_id_str(enum kernel_read_file_id 
> id)
>  {
> if (id < 0 || id >= READING_MAX_ID)
> return kernel_read_file_str[READING_UNKNOWN];
> --
> 2.6.3
>
>
> --
> Kees Cook
> Chrome OS & Brillo Security



-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] fs: fix over-zealous use of "const"

[Andy Shevchenko]

On Thu, Apr 21, 2016 at 10:53 PM, Kees Cook  wrote:
> When I was fixing up const recommendations from checkpatch.pl, I went
> overboard. This fixes the warning (during a W=1 build):
>
> include/linux/fs.h:2627:74: warning: type qualifiers ignored on function 
> return type [-Wignored-qualifiers]
> static inline const char * const kernel_read_file_id_str(enum 
> kernel_read_file_id id)
>
> Reported-by: Andy Shevchenko 
> Signed-off-by: Kees Cook 

Will test next week if someone don't do that before.
Thanks.

> ---
> This is for linux-security next
> ---
>  include/linux/fs.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 518716b4834e..82f9a2db3b1a 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2610,7 +2610,7 @@ static const char * const kernel_read_file_str[] = {
> __kernel_read_file_id(__fid_stringify)
>  };
>
> -static inline const char * const kernel_read_file_id_str(enum 
> kernel_read_file_id id)
> +static inline const char *kernel_read_file_id_str(enum kernel_read_file_id 
> id)
>  {
> if (id < 0 || id >= READING_MAX_ID)
> return kernel_read_file_str[READING_UNKNOWN];
> --
> 2.6.3
>
>
> --
> Kees Cook
> Chrome OS & Brillo Security



-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 01/19] clk: composite: Add unregister function

[Stephen Boyd]

On 04/19, Maxime Ripard wrote:
> 
> I have a bunch of other clock patches that need this, so I guess it
> would be easier if applied it directly with your acked-by, or if you
> could apply it and give a stable branch I can base my future PR on.

Ok. I pushed it to stable branch 'clk-composite-unregister' in
the clk.git repo. That way I can go ahead and delete this API
assuming I get around to converting all the composite users in
the next few days.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v3 01/19] clk: composite: Add unregister function

[Stephen Boyd]

On 04/19, Maxime Ripard wrote:
> 
> I have a bunch of other clock patches that need this, so I guess it
> would be easier if applied it directly with your acked-by, or if you
> could apply it and give a stable branch I can base my future PR on.

Ok. I pushed it to stable branch 'clk-composite-unregister' in
the clk.git repo. That way I can go ahead and delete this API
assuming I get around to converting all the composite users in
the next few days.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH] arch/x86/configs/x86_64_defconfig: enable CONFIG_E1000E=y

[Andy Shevchenko]

On Tue, Apr 19, 2016 at 11:21 AM, Konstantin Khlebnikov
 wrote:
> Very common ethernet. Already enabled in i386_defconfig

Noticed by disappearing same commit in my local tree, thanks.

FWIW:
Reviewed-and-tested-by: Andy Shevchenko 

>
> Signed-off-by: Konstantin Khlebnikov 
> ---
>  arch/x86/configs/x86_64_defconfig |1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/arch/x86/configs/x86_64_defconfig 
> b/arch/x86/configs/x86_64_defconfig
> index 4f404a64681b..0c8d7963483c 100644
> --- a/arch/x86/configs/x86_64_defconfig
> +++ b/arch/x86/configs/x86_64_defconfig
> @@ -173,6 +173,7 @@ CONFIG_TIGON3=y
>  CONFIG_NET_TULIP=y
>  CONFIG_E100=y
>  CONFIG_E1000=y
> +CONFIG_E1000E=y
>  CONFIG_SKY2=y
>  CONFIG_FORCEDETH=y
>  CONFIG_8139TOO=y
>



-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] arch/x86/configs/x86_64_defconfig: enable CONFIG_E1000E=y

[Andy Shevchenko]

On Tue, Apr 19, 2016 at 11:21 AM, Konstantin Khlebnikov
 wrote:
> Very common ethernet. Already enabled in i386_defconfig

Noticed by disappearing same commit in my local tree, thanks.

FWIW:
Reviewed-and-tested-by: Andy Shevchenko 

>
> Signed-off-by: Konstantin Khlebnikov 
> ---
>  arch/x86/configs/x86_64_defconfig |1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/arch/x86/configs/x86_64_defconfig 
> b/arch/x86/configs/x86_64_defconfig
> index 4f404a64681b..0c8d7963483c 100644
> --- a/arch/x86/configs/x86_64_defconfig
> +++ b/arch/x86/configs/x86_64_defconfig
> @@ -173,6 +173,7 @@ CONFIG_TIGON3=y
>  CONFIG_NET_TULIP=y
>  CONFIG_E100=y
>  CONFIG_E1000=y
> +CONFIG_E1000E=y
>  CONFIG_SKY2=y
>  CONFIG_FORCEDETH=y
>  CONFIG_8139TOO=y
>



-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] ARM64: dts: rockchip: add core dtsi file for RK3399 SoCs

[Heiko Stübner]

Hi Jay,

Am Donnerstag, 21. April 2016, 11:58:12 schrieb Jianqun Xu:
> This patch adds rk3399.dtsi for rk3399 found on Rockchip
> RK3399 SoCs, also add rk3399-evb.dts for Rockchip RK3399
> Evaluation Board.
> 
> Patch is tested on RK3399 evb.
> 
> Signed-off-by: Jianqun Xu 

please split this into
- patch adding the dtsi
- patch adding the evb dts
- patch adding the new  board to bindings/arm/rockchip.txt

more inline below

> ---
>  arch/arm64/boot/dts/rockchip/Makefile   |1 +
>  arch/arm64/boot/dts/rockchip/rk3399-evb.dts |  537 
>  arch/arm64/boot/dts/rockchip/rk3399.dtsi| 1757
> +++ 3 files changed, 2295 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/rockchip/rk3399-evb.dts
>  create mode 100644 arch/arm64/boot/dts/rockchip/rk3399.dtsi
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-evb.dts
> b/arch/arm64/boot/dts/rockchip/rk3399-evb.dts new file mode 100644
> index 000..4cb0028
> --- /dev/null
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-evb.dts
> @@ -0,0 +1,537 @@
> +/*
> + * Copyright (c) 2016 Fuzhou Rockchip Electronics Co., Ltd
> + *
> + * This file is dual-licensed: you can use it either under the terms
> + * of the GPL or the X11 license, at your option. Note that this dual
> + * licensing only applies to this file, and not this project as a
> + * whole.
> + *
> + *  a) This file is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of the
> + * License, or (at your option) any later version.
> + *
> + * This file is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * Or, alternatively,
> + *
> + *  b) Permission is hereby granted, free of charge, to any person
> + * obtaining a copy of this software and associated documentation
> + * files (the "Software"), to deal in the Software without
> + * restriction, including without limitation the rights to use,
> + * copy, modify, merge, publish, distribute, sublicense, and/or
> + * sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following
> + * conditions:
> + *
> + * The above copyright notice and this permission notice shall be
> + * included in all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
> + * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
> + * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
> + * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + * OTHER DEALINGS IN THE SOFTWARE.
> + */
> +
> +/dts-v1/;
> +
> +#include 
> +#include "rk3399.dtsi"
> +
> +/ {
> + model = "Rockchip RK3399 Evaluation Board";
> + compatible = "rockchip,evb", "rockchip,rk3399-evb";
> +
> + chosen {
> + bootargs = "console=uart,mmio32,0xff1a";

I'd think we'll want a
stdout-path = something
property here, instead of hard-coding bootargs.

[...]

> + {
> + status = "okay";
> + i2c-scl-rising-time-ns = <600>;
> + i2c-scl-falling-time-ns = <20>;
> +
> + gt9xx: gt9xx@14 {
> + compatible = "goodix,gt9xx";

same as Rob said for the ramoops, I don't see this one in the devicetree 
bindings. Also gt9xx should instead specify an actual chip, not a chip-family.
See drivers/input/touchscreen/goodix.c and 
Documentation/devicetree/bindings/input/touchscreen for supported chips and 
the real devicetree bindings.


> + reg = <0x14>;
> + touch-gpio = < 20 IRQ_TYPE_LEVEL_LOW>;
> + reset-gpio = < 22 GPIO_ACTIVE_HIGH>;
> + max-x = <1200>;
> + max-y = <1900>;
> + tp-size = <911>;
> + tp-supply = <_tp>;
> + };
> +};

[...]

> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> b/arch/arm64/boot/dts/rockchip/rk3399.dtsi new file mode 100644
> index 000..7c3015c
> --- /dev/null
> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> @@ -0,0 +1,1757 @@
> +/*
> + * Copyright (c) 2016 Fuzhou Rockchip Electronics Co., Ltd
> + *
> + * This file is dual-licensed: you can use it either under the terms
> + * of the GPL or the X11 license, at your option. Note that this dual
> + * licensing only applies to this file, and not this project as a
> + * whole.
> + *
> + *  a) This library is free software; you can redistribute it and/or
> + * modify it under the 

Re: [PATCH] ARM64: dts: rockchip: add core dtsi file for RK3399 SoCs

[Heiko Stübner]

Hi Jay,

Am Donnerstag, 21. April 2016, 11:58:12 schrieb Jianqun Xu:
> This patch adds rk3399.dtsi for rk3399 found on Rockchip
> RK3399 SoCs, also add rk3399-evb.dts for Rockchip RK3399
> Evaluation Board.
> 
> Patch is tested on RK3399 evb.
> 
> Signed-off-by: Jianqun Xu 

please split this into
- patch adding the dtsi
- patch adding the evb dts
- patch adding the new  board to bindings/arm/rockchip.txt

more inline below

> ---
>  arch/arm64/boot/dts/rockchip/Makefile   |1 +
>  arch/arm64/boot/dts/rockchip/rk3399-evb.dts |  537 
>  arch/arm64/boot/dts/rockchip/rk3399.dtsi| 1757
> +++ 3 files changed, 2295 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/rockchip/rk3399-evb.dts
>  create mode 100644 arch/arm64/boot/dts/rockchip/rk3399.dtsi
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-evb.dts
> b/arch/arm64/boot/dts/rockchip/rk3399-evb.dts new file mode 100644
> index 000..4cb0028
> --- /dev/null
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-evb.dts
> @@ -0,0 +1,537 @@
> +/*
> + * Copyright (c) 2016 Fuzhou Rockchip Electronics Co., Ltd
> + *
> + * This file is dual-licensed: you can use it either under the terms
> + * of the GPL or the X11 license, at your option. Note that this dual
> + * licensing only applies to this file, and not this project as a
> + * whole.
> + *
> + *  a) This file is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of the
> + * License, or (at your option) any later version.
> + *
> + * This file is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * Or, alternatively,
> + *
> + *  b) Permission is hereby granted, free of charge, to any person
> + * obtaining a copy of this software and associated documentation
> + * files (the "Software"), to deal in the Software without
> + * restriction, including without limitation the rights to use,
> + * copy, modify, merge, publish, distribute, sublicense, and/or
> + * sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following
> + * conditions:
> + *
> + * The above copyright notice and this permission notice shall be
> + * included in all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
> + * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
> + * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
> + * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + * OTHER DEALINGS IN THE SOFTWARE.
> + */
> +
> +/dts-v1/;
> +
> +#include 
> +#include "rk3399.dtsi"
> +
> +/ {
> + model = "Rockchip RK3399 Evaluation Board";
> + compatible = "rockchip,evb", "rockchip,rk3399-evb";
> +
> + chosen {
> + bootargs = "console=uart,mmio32,0xff1a";

I'd think we'll want a
stdout-path = something
property here, instead of hard-coding bootargs.

[...]

> + {
> + status = "okay";
> + i2c-scl-rising-time-ns = <600>;
> + i2c-scl-falling-time-ns = <20>;
> +
> + gt9xx: gt9xx@14 {
> + compatible = "goodix,gt9xx";

same as Rob said for the ramoops, I don't see this one in the devicetree 
bindings. Also gt9xx should instead specify an actual chip, not a chip-family.
See drivers/input/touchscreen/goodix.c and 
Documentation/devicetree/bindings/input/touchscreen for supported chips and 
the real devicetree bindings.


> + reg = <0x14>;
> + touch-gpio = < 20 IRQ_TYPE_LEVEL_LOW>;
> + reset-gpio = < 22 GPIO_ACTIVE_HIGH>;
> + max-x = <1200>;
> + max-y = <1900>;
> + tp-size = <911>;
> + tp-supply = <_tp>;
> + };
> +};

[...]

> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> b/arch/arm64/boot/dts/rockchip/rk3399.dtsi new file mode 100644
> index 000..7c3015c
> --- /dev/null
> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> @@ -0,0 +1,1757 @@
> +/*
> + * Copyright (c) 2016 Fuzhou Rockchip Electronics Co., Ltd
> + *
> + * This file is dual-licensed: you can use it either under the terms
> + * of the GPL or the X11 license, at your option. Note that this dual
> + * licensing only applies to this file, and not this project as a
> + * whole.
> + *
> + *  a) This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General 

Re: [PATCH V2 14/15] coresight: tmc: implementing TMC-ETR AUX space API

[Mathieu Poirier]

On 21 April 2016 at 10:10, Suzuki K Poulose  wrote:
> On 12/04/16 18:54, Mathieu Poirier wrote:
>>
>> This patch implement the AUX area interfaces required to
>> use the TMC (configured as an ETR) from the Perf sub-system.
>>
>> The heuristic is heavily borrowed from the ETB10 and TMC-ETF
>> implementation.
>>
>> Signed-off-by: Mathieu Poirier 
>
>
>> +static void tmc_update_etr_buffer(struct coresight_device *csdev,
>> + struct perf_output_handle *handle,
>> + void *sink_config)
>> +{
>> +   struct cs_etr_buffers *buf = sink_config;
>> +
>> +   /*
>> +* An ETR configured to work in contiguous memory mode works the
>> same
>> +* was as an ETB or ETF.
>> +*/
>> +   tmc_update_etf_buffer(csdev, handle, >tmc);
>
>
> Really ? I thought the ETR stores the data to the allocated System RAM and
> can
> be read directly from the memory than using the RRD ?

Using an ETR in contiguous mode is inefficient to start with.
Regardless of the approach taken trace data has to be copied to perf's
non-contiguous ring buffer pages.  It would be more efficient to copy
bigger chunks of data to the ring buffer (because we can), but I
rather spend my time enabling scatter-gather mode than optimizing a
sub-optimal mode of operation.

This currently work and enables people to use the IP block while
scatter-gather mode is in flight.  I should probably make that clear
in the commit log.

Thanks,
Mathieu

>
> Suzuki
>
>
>

Re: [PATCH V2 14/15] coresight: tmc: implementing TMC-ETR AUX space API

[Mathieu Poirier]

On 21 April 2016 at 10:10, Suzuki K Poulose  wrote:
> On 12/04/16 18:54, Mathieu Poirier wrote:
>>
>> This patch implement the AUX area interfaces required to
>> use the TMC (configured as an ETR) from the Perf sub-system.
>>
>> The heuristic is heavily borrowed from the ETB10 and TMC-ETF
>> implementation.
>>
>> Signed-off-by: Mathieu Poirier 
>
>
>> +static void tmc_update_etr_buffer(struct coresight_device *csdev,
>> + struct perf_output_handle *handle,
>> + void *sink_config)
>> +{
>> +   struct cs_etr_buffers *buf = sink_config;
>> +
>> +   /*
>> +* An ETR configured to work in contiguous memory mode works the
>> same
>> +* was as an ETB or ETF.
>> +*/
>> +   tmc_update_etf_buffer(csdev, handle, >tmc);
>
>
> Really ? I thought the ETR stores the data to the allocated System RAM and
> can
> be read directly from the memory than using the RRD ?

Using an ETR in contiguous mode is inefficient to start with.
Regardless of the approach taken trace data has to be copied to perf's
non-contiguous ring buffer pages.  It would be more efficient to copy
bigger chunks of data to the ring buffer (because we can), but I
rather spend my time enabling scatter-gather mode than optimizing a
sub-optimal mode of operation.

This currently work and enables people to use the IP block while
scatter-gather mode is in flight.  I should probably make that clear
in the commit log.

Thanks,
Mathieu

>
> Suzuki
>
>
>

Re: [PATCH v6] i2c: rk3x: add i2c support for rk3399 soc

[Andy Shevchenko]

On Tue, Apr 19, 2016 at 4:55 PM, David Wu  wrote:
> From: David Wu 
>
> - new method to caculate i2c timings for rk3399:
>   There was an timing issue about "repeated start" time at the I2C
>   controller of version0, controller appears to drop SDA at .875x (7/8)
>   programmed clk high. On version 1 of the controller, the rule(.875x)
>   isn't enough to meet tSU;STA
>   requirements on 100k's Standard-mode. To resolve this issue,
>   sda_update_config, start_setup_config and stop_setup_config for I2C
>   timing information are added, new rules are designed to calculate
>   the timing information at new v1.
> - pclk and function clk are separated at rk3399.
> - support i2c highspeed mode: 1.7MHz for rk3399
>

Remove redundant parens in cases like
1. x * (a->b)
2. x = (a + b);

Don't touch pieces of code that are not related (you removed one empty
line without any reason).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v6] i2c: rk3x: add i2c support for rk3399 soc

[Andy Shevchenko]

On Tue, Apr 19, 2016 at 4:55 PM, David Wu  wrote:
> From: David Wu 
>
> - new method to caculate i2c timings for rk3399:
>   There was an timing issue about "repeated start" time at the I2C
>   controller of version0, controller appears to drop SDA at .875x (7/8)
>   programmed clk high. On version 1 of the controller, the rule(.875x)
>   isn't enough to meet tSU;STA
>   requirements on 100k's Standard-mode. To resolve this issue,
>   sda_update_config, start_setup_config and stop_setup_config for I2C
>   timing information are added, new rules are designed to calculate
>   the timing information at new v1.
> - pclk and function clk are separated at rk3399.
> - support i2c highspeed mode: 1.7MHz for rk3399
>

Remove redundant parens in cases like
1. x * (a->b)
2. x = (a + b);

Don't touch pieces of code that are not related (you removed one empty
line without any reason).

-- 
With Best Regards,
Andy Shevchenko

[PATCH] HID: uhid: Fixes a bug with userspace bluetooth stacks, which causes hangs during certain operations v2

[roderick]

From: Roderick Colenbrander 

Many devices use userspace bluetooth stacks like BlueZ or Bluedroid in 
combination
with uhid. If any of these stacks is used with a HID device for which the driver
performs a HID request as part .probe (or technically another HID operation),
this results in a deadlock situation. The deadlock results in a 5 second timeout
for I/O operations in HID drivers, so isn't fatal, but none of the I/O 
operations
have a chance of succeeding.

The root cause for the problem is that uhid only allows for one request to be
processed at a time per uhid instance and locks out other operations. This means
that if a user space is creating a new HID device through 'UHID_CREATE', which
ultimately triggers '.probe' through the HID layer. Then any HID request e.g. a
read for calibration data would trigger a HID operation on uhid again, but it
won't go out to userspace, because it is still stuck in UHID_CREATE.
In addition bluetooth stacks are typically single threaded, so they wouldn't be
able to handle any requests while waiting on uhid.

Lucikly the UHID spec is somewhat flexible and allows for fixing the issue,
without breaking user space. The idea which the patch implements as discussed
with David Herrmann is to decouple adding of a hid device (which triggers 
.probe)
from UHID_CREATE. The work will kick off roughly once UHID_CREATE completed (or
else will wait a tiny bit of time in .probe for a lock). A HID driver has to 
call
HID to call 'hid_hw_start()' as part of .probe once it is ready for I/O, which
triggers UHID_START to user space. Any HID operations should function now within
.probe and won't deadlock because userspace is stuck on UHID_CREATE.

We verified this patch on Bluedroid with Android 6.0 and on desktop Linux with
BlueZ stacks. Prior to the patch they had the deadlock issue.

Signed-off-by: Roderick Colenbrander 
Cc: sta...@vger.kernel.org
---
 drivers/hid/uhid.c | 33 -
 1 file changed, 24 insertions(+), 9 deletions(-)

diff --git a/drivers/hid/uhid.c b/drivers/hid/uhid.c
index 16b6f11..99ec3ff 100644
--- a/drivers/hid/uhid.c
+++ b/drivers/hid/uhid.c
@@ -51,10 +51,26 @@ struct uhid_device {
u32 report_id;
u32 report_type;
struct uhid_event report_buf;
+   struct work_struct worker;
 };
 
 static struct miscdevice uhid_misc;
 
+static void uhid_device_add_worker(struct work_struct *work)
+{
+   struct uhid_device *uhid = container_of(work, struct uhid_device, 
worker);
+   int ret;
+
+   ret = hid_add_device(uhid->hid);
+   if (ret) {
+   hid_err(uhid->hid, "Cannot register HID device: error %d\n", 
ret);
+
+   hid_destroy_device(uhid->hid);
+   uhid->hid = NULL;
+   uhid->running = false;
+   }
+}
+
 static void uhid_queue(struct uhid_device *uhid, struct uhid_event *ev)
 {
__u8 newhead;
@@ -498,18 +514,14 @@ static int uhid_dev_create2(struct uhid_device *uhid,
uhid->hid = hid;
uhid->running = true;
 
-   ret = hid_add_device(hid);
-   if (ret) {
-   hid_err(hid, "Cannot register HID device\n");
-   goto err_hid;
-   }
+   /* Adding of a HID device is done through a worker, to allow HID drivers
+* which use feature requests during .probe to work, without they would
+* be blocked on devlock, which is held by uhid_char_write.
+*/
+   schedule_work(>worker);
 
return 0;
 
-err_hid:
-   hid_destroy_device(hid);
-   uhid->hid = NULL;
-   uhid->running = false;
 err_free:
kfree(uhid->rd_data);
uhid->rd_data = NULL;
@@ -550,6 +562,8 @@ static int uhid_dev_destroy(struct uhid_device *uhid)
uhid->running = false;
wake_up_interruptible(>report_wait);
 
+   cancel_work_sync(>worker);
+
hid_destroy_device(uhid->hid);
kfree(uhid->rd_data);
 
@@ -612,6 +626,7 @@ static int uhid_char_open(struct inode *inode, struct file 
*file)
init_waitqueue_head(>waitq);
init_waitqueue_head(>report_wait);
uhid->running = false;
+   INIT_WORK(>worker, uhid_device_add_worker);
 
file->private_data = uhid;
nonseekable_open(inode, file);
-- 
2.5.5

[PATCH] HID: uhid: Fixes a bug with userspace bluetooth stacks, which causes hangs during certain operations v2

[roderick]

From: Roderick Colenbrander 

Many devices use userspace bluetooth stacks like BlueZ or Bluedroid in 
combination
with uhid. If any of these stacks is used with a HID device for which the driver
performs a HID request as part .probe (or technically another HID operation),
this results in a deadlock situation. The deadlock results in a 5 second timeout
for I/O operations in HID drivers, so isn't fatal, but none of the I/O 
operations
have a chance of succeeding.

The root cause for the problem is that uhid only allows for one request to be
processed at a time per uhid instance and locks out other operations. This means
that if a user space is creating a new HID device through 'UHID_CREATE', which
ultimately triggers '.probe' through the HID layer. Then any HID request e.g. a
read for calibration data would trigger a HID operation on uhid again, but it
won't go out to userspace, because it is still stuck in UHID_CREATE.
In addition bluetooth stacks are typically single threaded, so they wouldn't be
able to handle any requests while waiting on uhid.

Lucikly the UHID spec is somewhat flexible and allows for fixing the issue,
without breaking user space. The idea which the patch implements as discussed
with David Herrmann is to decouple adding of a hid device (which triggers 
.probe)
from UHID_CREATE. The work will kick off roughly once UHID_CREATE completed (or
else will wait a tiny bit of time in .probe for a lock). A HID driver has to 
call
HID to call 'hid_hw_start()' as part of .probe once it is ready for I/O, which
triggers UHID_START to user space. Any HID operations should function now within
.probe and won't deadlock because userspace is stuck on UHID_CREATE.

We verified this patch on Bluedroid with Android 6.0 and on desktop Linux with
BlueZ stacks. Prior to the patch they had the deadlock issue.

Signed-off-by: Roderick Colenbrander 
Cc: sta...@vger.kernel.org
---
 drivers/hid/uhid.c | 33 -
 1 file changed, 24 insertions(+), 9 deletions(-)

diff --git a/drivers/hid/uhid.c b/drivers/hid/uhid.c
index 16b6f11..99ec3ff 100644
--- a/drivers/hid/uhid.c
+++ b/drivers/hid/uhid.c
@@ -51,10 +51,26 @@ struct uhid_device {
u32 report_id;
u32 report_type;
struct uhid_event report_buf;
+   struct work_struct worker;
 };
 
 static struct miscdevice uhid_misc;
 
+static void uhid_device_add_worker(struct work_struct *work)
+{
+   struct uhid_device *uhid = container_of(work, struct uhid_device, 
worker);
+   int ret;
+
+   ret = hid_add_device(uhid->hid);
+   if (ret) {
+   hid_err(uhid->hid, "Cannot register HID device: error %d\n", 
ret);
+
+   hid_destroy_device(uhid->hid);
+   uhid->hid = NULL;
+   uhid->running = false;
+   }
+}
+
 static void uhid_queue(struct uhid_device *uhid, struct uhid_event *ev)
 {
__u8 newhead;
@@ -498,18 +514,14 @@ static int uhid_dev_create2(struct uhid_device *uhid,
uhid->hid = hid;
uhid->running = true;
 
-   ret = hid_add_device(hid);
-   if (ret) {
-   hid_err(hid, "Cannot register HID device\n");
-   goto err_hid;
-   }
+   /* Adding of a HID device is done through a worker, to allow HID drivers
+* which use feature requests during .probe to work, without they would
+* be blocked on devlock, which is held by uhid_char_write.
+*/
+   schedule_work(>worker);
 
return 0;
 
-err_hid:
-   hid_destroy_device(hid);
-   uhid->hid = NULL;
-   uhid->running = false;
 err_free:
kfree(uhid->rd_data);
uhid->rd_data = NULL;
@@ -550,6 +562,8 @@ static int uhid_dev_destroy(struct uhid_device *uhid)
uhid->running = false;
wake_up_interruptible(>report_wait);
 
+   cancel_work_sync(>worker);
+
hid_destroy_device(uhid->hid);
kfree(uhid->rd_data);
 
@@ -612,6 +626,7 @@ static int uhid_char_open(struct inode *inode, struct file 
*file)
init_waitqueue_head(>waitq);
init_waitqueue_head(>report_wait);
uhid->running = false;
+   INIT_WORK(>worker, uhid_device_add_worker);
 
file->private_data = uhid;
nonseekable_open(inode, file);
-- 
2.5.5

Re: [PATCH] ARM64: dts: rockchip: add core dtsi file for rk3399 SoCs

[Brian Norris]

Hi,

On Wed, Apr 20, 2016 at 11:15:50AM +0800, Jianqun Xu wrote:
> This patch adds core dtsi file for rk3399 found on Rockchip
> rk3399 SoCs, tested on rk3399 evb.
> 
> Signed-off-by: Jianqun Xu 
> ---
>  arch/arm64/boot/dts/rockchip/rk3399.dtsi | 1744 
> ++
>  1 file changed, 1744 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/rockchip/rk3399.dtsi
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi 
> b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> new file mode 100644
> index 000..fa6fc2c
> --- /dev/null
> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> @@ -0,0 +1,1744 @@
[...]

> + emmc_phy: phy {
> + compatible = "rockchip,rk3399-emmc-phy";
> + reg-offset = <0xf780>;

This property is not documented. The current doc says we "require" reg,
but you're kinda misusing it, I believe. At any rate, the current phy
driver won't probe without 'reg'.

> + #phy-cells = <0>;
> + rockchip,grf = <>;
> + status = "disabled";
> + };

[...]

Brian

Re: [PATCH] ARM64: dts: rockchip: add core dtsi file for rk3399 SoCs

[Brian Norris]

Hi,

On Wed, Apr 20, 2016 at 11:15:50AM +0800, Jianqun Xu wrote:
> This patch adds core dtsi file for rk3399 found on Rockchip
> rk3399 SoCs, tested on rk3399 evb.
> 
> Signed-off-by: Jianqun Xu 
> ---
>  arch/arm64/boot/dts/rockchip/rk3399.dtsi | 1744 
> ++
>  1 file changed, 1744 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/rockchip/rk3399.dtsi
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi 
> b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> new file mode 100644
> index 000..fa6fc2c
> --- /dev/null
> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> @@ -0,0 +1,1744 @@
[...]

> + emmc_phy: phy {
> + compatible = "rockchip,rk3399-emmc-phy";
> + reg-offset = <0xf780>;

This property is not documented. The current doc says we "require" reg,
but you're kinda misusing it, I believe. At any rate, the current phy
driver won't probe without 'reg'.

> + #phy-cells = <0>;
> + rockchip,grf = <>;
> + status = "disabled";
> + };

[...]

Brian

Re: [RFC PATCH v2 05/10] i2c: add support for ACPI reconfigure notifications

[Octavian Purdila]

On Fri, Apr 22, 2016 at 12:27 AM, Andy Shevchenko
 wrote:
>

Hi Andy and thanks for the review!

> On Wed, Apr 20, 2016 at 1:39 AM, Octavian Purdila
>  wrote:
> > This patch adds supports for I2C device enumeration and removal via
> > ACPI reconfiguration notifications that are send as a result of an
> > ACPI table load or unload operation.
>
> >
> > acpi_dev_free_resource_list(_list);
> >
> > +   strlcpy(info->type, dev_name(>dev), sizeof(info->type));
>
> strscpy() ?

That is the original code, I just moved it in a different function.
Its probably best if we use a separate patch for this, but is it worth
it?

>
> > +static int acpi_i2c_match_adapter(struct device *dev, void *data)
> > +{
> > +   struct i2c_adapter *adapter = i2c_verify_adapter(dev);
> > +
> > +   if (!adapter)
> > +   return 0;
> > +
>
> > +   return ACPI_HANDLE(dev) == (acpi_handle)data;
>
> What is suppose to return? Hidden bool to integer conversion is not
> best option I suppose.
>

per bus_find_device() :

 * The callback should return 0 if the device doesn't match and
non-zero
 * if it does.  If the callback returns non-zero, this function will
 * return to the caller and not iterate over any more devices.
 */
struct device *bus_find_device(struct bus_type *bus,
   struct device *start, void *data,
   int (*match)(struct device *dev, void *data))


> > +}
> > +
> > +static int acpi_i2c_match_device(struct device *dev, void *data)
> > +{
> > +   return ACPI_COMPANION(dev) == data;
>
> Ditto.
>
> > +}
>
> --
> With Best Regards,
> Andy Shevchenko

Re: [RFC PATCH v2 05/10] i2c: add support for ACPI reconfigure notifications

[Octavian Purdila]

On Fri, Apr 22, 2016 at 12:27 AM, Andy Shevchenko
 wrote:
>

Hi Andy and thanks for the review!

> On Wed, Apr 20, 2016 at 1:39 AM, Octavian Purdila
>  wrote:
> > This patch adds supports for I2C device enumeration and removal via
> > ACPI reconfiguration notifications that are send as a result of an
> > ACPI table load or unload operation.
>
> >
> > acpi_dev_free_resource_list(_list);
> >
> > +   strlcpy(info->type, dev_name(>dev), sizeof(info->type));
>
> strscpy() ?

That is the original code, I just moved it in a different function.
Its probably best if we use a separate patch for this, but is it worth
it?

>
> > +static int acpi_i2c_match_adapter(struct device *dev, void *data)
> > +{
> > +   struct i2c_adapter *adapter = i2c_verify_adapter(dev);
> > +
> > +   if (!adapter)
> > +   return 0;
> > +
>
> > +   return ACPI_HANDLE(dev) == (acpi_handle)data;
>
> What is suppose to return? Hidden bool to integer conversion is not
> best option I suppose.
>

per bus_find_device() :

 * The callback should return 0 if the device doesn't match and
non-zero
 * if it does.  If the callback returns non-zero, this function will
 * return to the caller and not iterate over any more devices.
 */
struct device *bus_find_device(struct bus_type *bus,
   struct device *start, void *data,
   int (*match)(struct device *dev, void *data))


> > +}
> > +
> > +static int acpi_i2c_match_device(struct device *dev, void *data)
> > +{
> > +   return ACPI_COMPANION(dev) == data;
>
> Ditto.
>
> > +}
>
> --
> With Best Regards,
> Andy Shevchenko

[PATCH net-next] hv_netvsc: Fix the list processing for network change event

[Haiyang Zhang]

RNDIS_STATUS_NETWORK_CHANGE event is handled as two "half events" --
media disconnect & connect. The second half should be added to the list
head, not to the tail. So all events are processed in normal order.

Signed-off-by: Haiyang Zhang 
Reviewed-by: K. Y. Srinivasan 
---
 drivers/net/hyperv/netvsc_drv.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index bfdb568a..ba3f3f3 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -1125,7 +1125,7 @@ static void netvsc_link_change(struct work_struct *w)
netif_tx_stop_all_queues(net);
event->event = RNDIS_STATUS_MEDIA_CONNECT;
spin_lock_irqsave(_ctx->lock, flags);
-   list_add_tail(>list, _ctx->reconfig_events);
+   list_add(>list, _ctx->reconfig_events);
spin_unlock_irqrestore(_ctx->lock, flags);
reschedule = true;
}
-- 
1.7.4.1

[PATCH net-next] hv_netvsc: Fix the list processing for network change event

[Haiyang Zhang]

RNDIS_STATUS_NETWORK_CHANGE event is handled as two "half events" --
media disconnect & connect. The second half should be added to the list
head, not to the tail. So all events are processed in normal order.

Signed-off-by: Haiyang Zhang 
Reviewed-by: K. Y. Srinivasan 
---
 drivers/net/hyperv/netvsc_drv.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index bfdb568a..ba3f3f3 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -1125,7 +1125,7 @@ static void netvsc_link_change(struct work_struct *w)
netif_tx_stop_all_queues(net);
event->event = RNDIS_STATUS_MEDIA_CONNECT;
spin_lock_irqsave(_ctx->lock, flags);
-   list_add_tail(>list, _ctx->reconfig_events);
+   list_add(>list, _ctx->reconfig_events);
spin_unlock_irqrestore(_ctx->lock, flags);
reschedule = true;
}
-- 
1.7.4.1

Re: [PATCH v6 07/19] i2c: octeon: Use i2c recovery framework

[Wolfram Sang]

> I would like to disable SMBUS_QUICK. It never worked for the read case.

Fine with me.

> Could we break something by disabling the quick-write case or is
> the quick-write emulated by a larger write if the feature bit is not
> set?

No emulation. It is simply not supported then. It is actually quite
dangerous to simulate it with regular write.



signature.asc
Description: PGP signature

Re: [PATCH v6 07/19] i2c: octeon: Use i2c recovery framework

[Wolfram Sang]

> I would like to disable SMBUS_QUICK. It never worked for the read case.

Fine with me.

> Could we break something by disabling the quick-write case or is
> the quick-write emulated by a larger write if the feature bit is not
> set?

No emulation. It is simply not supported then. It is actually quite
dangerous to simulate it with regular write.



signature.asc
Description: PGP signature

Re: Nokia N900 retention mode in v4.6, camera buttons fun

[Pavel Machek]

Hi!

> > CONFIG_HSI breaks power management completely, so power management
> > with modem will be another topic.  
> 
> Sebastian, any idea why power management does not work for HSI?
> 
> > > > > In the meantime, I found what is causing the rention mode to break for
> > > > > me: CONFIG_HSI (aka wireless modem support). With HSI off, it seems 
> > > > > to work.
> > > > > 
> > > > > I still get problems with the camera button, in config similar to
> > > > > defconfig.  For some reason, I'm even getting (autorepeating) ^@ on
> > > > > console. As long as I hold camera button down, I even get it into off
> > > > > mode for brief period.
> > > > 
> > > > Ok, if I turn off CONFIG_KEYBOARD_GPIO, I get it into off
> > > > mode... once per screen blank, for about a second. (Does 
> > > > CONFIG_KEYBOARD_GPIO also cause
> > > > problems for you?)
> > > > 
> > > > Any idea why it enters off mode only once after each screenblank?
> > > 
> > > After disabling CONFIG_PROVE_LOCKING, loading the LCD modules, and
> > > blanking the screen, my n900 hits off mode just fine about once
> > > a second. Sounds like you still have some extra devices enabled
> > > causing it.
> > 
> > I checked again... also with vanilla 4.6-rc2 to double check... same effect.
> > 
> > Aha, got it... cat-ing /sys/kernel/debug/pm_debug/count breaks the
> > off mode. If I don't do that (tm), it seems to work way better.
> 
> So what is result? Is power management working for CONFIG_KEYBOARD_GPIO?

camera and unlock button GPIOs seem to break the powermanagement,
too. I disabled it for now.

Next hint I got from Sebastian was that I may need to enable power
management in /sys.

pavel@n900:/my/tui/ofone$ cat
/sys/devices/platform/6800.ocp/48058000.ssi-controller/power/runtime_status
active
pavel@n900:/my/tui/ofone$ cat
/sys/devices/platform/6800.ocp/48058000.ssi-controller/power/autosuspend_delay_ms
cat:
/sys/devices/platform/6800.ocp/48058000.ssi-controller/power/autosuspend_delay_ms:
Input/output error
root@n900:/my/tui/ofone# cat
/sys/devices/platform/6800.ocp/48058000.ssi-controller/power/control
auto

I could not get it to sleep :-(.

Best regards,
Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: Nokia N900 retention mode in v4.6, camera buttons fun

[Pavel Machek]

Hi!

> > CONFIG_HSI breaks power management completely, so power management
> > with modem will be another topic.  
> 
> Sebastian, any idea why power management does not work for HSI?
> 
> > > > > In the meantime, I found what is causing the rention mode to break for
> > > > > me: CONFIG_HSI (aka wireless modem support). With HSI off, it seems 
> > > > > to work.
> > > > > 
> > > > > I still get problems with the camera button, in config similar to
> > > > > defconfig.  For some reason, I'm even getting (autorepeating) ^@ on
> > > > > console. As long as I hold camera button down, I even get it into off
> > > > > mode for brief period.
> > > > 
> > > > Ok, if I turn off CONFIG_KEYBOARD_GPIO, I get it into off
> > > > mode... once per screen blank, for about a second. (Does 
> > > > CONFIG_KEYBOARD_GPIO also cause
> > > > problems for you?)
> > > > 
> > > > Any idea why it enters off mode only once after each screenblank?
> > > 
> > > After disabling CONFIG_PROVE_LOCKING, loading the LCD modules, and
> > > blanking the screen, my n900 hits off mode just fine about once
> > > a second. Sounds like you still have some extra devices enabled
> > > causing it.
> > 
> > I checked again... also with vanilla 4.6-rc2 to double check... same effect.
> > 
> > Aha, got it... cat-ing /sys/kernel/debug/pm_debug/count breaks the
> > off mode. If I don't do that (tm), it seems to work way better.
> 
> So what is result? Is power management working for CONFIG_KEYBOARD_GPIO?

camera and unlock button GPIOs seem to break the powermanagement,
too. I disabled it for now.

Next hint I got from Sebastian was that I may need to enable power
management in /sys.

pavel@n900:/my/tui/ofone$ cat
/sys/devices/platform/6800.ocp/48058000.ssi-controller/power/runtime_status
active
pavel@n900:/my/tui/ofone$ cat
/sys/devices/platform/6800.ocp/48058000.ssi-controller/power/autosuspend_delay_ms
cat:
/sys/devices/platform/6800.ocp/48058000.ssi-controller/power/autosuspend_delay_ms:
Input/output error
root@n900:/my/tui/ofone# cat
/sys/devices/platform/6800.ocp/48058000.ssi-controller/power/control
auto

I could not get it to sleep :-(.

Best regards,
Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH] HID: uhid: Fixes a bug with userspace bluetooth stacks, which causes hangs during certain operations

[Roderick Colenbrander]

Please ignore this revision of the patch, there was a small merge
conflict on my end.

On Thu, Apr 21, 2016 at 2:15 PM, kbuild test robot <l...@intel.com> wrote:
> Hi,
>
> [auto build test WARNING on hid/for-next]
> [also build test WARNING on v4.6-rc4 next-20160421]
> [if your patch is applied to the wrong git tree, please drop us a note to 
> help improving the system]
>
> url:
> https://github.com/0day-ci/linux/commits/roderick-gaikai-com/HID-uhid-Fixes-a-bug-with-userspace-bluetooth-stacks-which-causes-hangs-during-certain-operations/20160422-050505
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/jikos/hid.git for-next
> config: x86_64-randconfig-x003-201616 (attached as .config)
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=x86_64
>
> All warnings (new ones prefixed by >>):
>
>In file included from arch/x86/include/asm/atomic.h:4:0,
> from include/linux/atomic.h:4,
> from drivers/hid/uhid.c:13:
>drivers/hid/uhid.c: In function 'uhid_event_from_user':
>drivers/hid/uhid.c:403:6: error: implicit declaration of function 
> 'is_compat_task' [-Werror=implicit-function-declaration]
>  if (is_compat_task()) {
>  ^
>include/linux/compiler.h:151:30: note: in definition of macro '__trace_if'
>  if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
>  ^
>>> drivers/hid/uhid.c:403:2: note: in expansion of macro 'if'
>  if (is_compat_task()) {
>  ^
>cc1: some warnings being treated as errors
>
> vim +/if +403 drivers/hid/uhid.c
>
>387  __u8 phys[64];
>388  __u8 uniq[64];
>389
>390  compat_uptr_t rd_data;
>391  __u16 rd_size;
>392
>393  __u16 bus;
>394  __u32 vendor;
>395  __u32 product;
>396  __u32 version;
>397  __u32 country;
>398  } __attribute__((__packed__));
>399
>400  static int uhid_event_from_user(const char __user *buffer, size_t len,
>401  struct uhid_event *event)
>402  {
>  > 403  if (is_compat_task()) {
>404  u32 type;
>405
>406  if (get_user(type, buffer))
>407  return -EFAULT;
>408
>409  if (type == UHID_CREATE) {
>410  /*
>411   * This is our messed up request with compat 
> pointer.
>
> ---
> 0-DAY kernel test infrastructureOpen Source Technology Center
> https://lists.01.org/pipermail/kbuild-all   Intel Corporation



-- 
Roderick Colenbrander
Manager of Software Engineering
Gaikai, a Sony Computer Entertainment Company
roder...@gaikai.com

Re: [PATCH] HID: uhid: Fixes a bug with userspace bluetooth stacks, which causes hangs during certain operations

[Roderick Colenbrander]

Please ignore this revision of the patch, there was a small merge
conflict on my end.

On Thu, Apr 21, 2016 at 2:15 PM, kbuild test robot  wrote:
> Hi,
>
> [auto build test WARNING on hid/for-next]
> [also build test WARNING on v4.6-rc4 next-20160421]
> [if your patch is applied to the wrong git tree, please drop us a note to 
> help improving the system]
>
> url:
> https://github.com/0day-ci/linux/commits/roderick-gaikai-com/HID-uhid-Fixes-a-bug-with-userspace-bluetooth-stacks-which-causes-hangs-during-certain-operations/20160422-050505
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/jikos/hid.git for-next
> config: x86_64-randconfig-x003-201616 (attached as .config)
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=x86_64
>
> All warnings (new ones prefixed by >>):
>
>In file included from arch/x86/include/asm/atomic.h:4:0,
> from include/linux/atomic.h:4,
> from drivers/hid/uhid.c:13:
>drivers/hid/uhid.c: In function 'uhid_event_from_user':
>drivers/hid/uhid.c:403:6: error: implicit declaration of function 
> 'is_compat_task' [-Werror=implicit-function-declaration]
>  if (is_compat_task()) {
>  ^
>include/linux/compiler.h:151:30: note: in definition of macro '__trace_if'
>  if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
>  ^
>>> drivers/hid/uhid.c:403:2: note: in expansion of macro 'if'
>  if (is_compat_task()) {
>  ^
>cc1: some warnings being treated as errors
>
> vim +/if +403 drivers/hid/uhid.c
>
>387  __u8 phys[64];
>388  __u8 uniq[64];
>389
>390  compat_uptr_t rd_data;
>391  __u16 rd_size;
>392
>393  __u16 bus;
>394  __u32 vendor;
>395  __u32 product;
>396  __u32 version;
>397  __u32 country;
>398  } __attribute__((__packed__));
>399
>400  static int uhid_event_from_user(const char __user *buffer, size_t len,
>401  struct uhid_event *event)
>402  {
>  > 403  if (is_compat_task()) {
>404  u32 type;
>405
>406  if (get_user(type, buffer))
>407  return -EFAULT;
>408
>409  if (type == UHID_CREATE) {
>410  /*
>411   * This is our messed up request with compat 
> pointer.
>
> ---
> 0-DAY kernel test infrastructureOpen Source Technology Center
> https://lists.01.org/pipermail/kbuild-all   Intel Corporation



-- 
Roderick Colenbrander
Manager of Software Engineering
Gaikai, a Sony Computer Entertainment Company
roder...@gaikai.com

Re: [RFC PATCH v2 05/10] i2c: add support for ACPI reconfigure notifications

[Andy Shevchenko]

On Wed, Apr 20, 2016 at 1:39 AM, Octavian Purdila
 wrote:
> This patch adds supports for I2C device enumeration and removal via
> ACPI reconfiguration notifications that are send as a result of an
> ACPI table load or unload operation.

>
> acpi_dev_free_resource_list(_list);
>
> +   strlcpy(info->type, dev_name(>dev), sizeof(info->type));

strscpy() ?

> +static int acpi_i2c_match_adapter(struct device *dev, void *data)
> +{
> +   struct i2c_adapter *adapter = i2c_verify_adapter(dev);
> +
> +   if (!adapter)
> +   return 0;
> +

> +   return ACPI_HANDLE(dev) == (acpi_handle)data;

What is suppose to return? Hidden bool to integer conversion is not
best option I suppose.

> +}
> +
> +static int acpi_i2c_match_device(struct device *dev, void *data)
> +{
> +   return ACPI_COMPANION(dev) == data;

Ditto.

> +}

-- 
With Best Regards,
Andy Shevchenko

Re: [RFC PATCH v2 05/10] i2c: add support for ACPI reconfigure notifications

[Andy Shevchenko]

On Wed, Apr 20, 2016 at 1:39 AM, Octavian Purdila
 wrote:
> This patch adds supports for I2C device enumeration and removal via
> ACPI reconfiguration notifications that are send as a result of an
> ACPI table load or unload operation.

>
> acpi_dev_free_resource_list(_list);
>
> +   strlcpy(info->type, dev_name(>dev), sizeof(info->type));

strscpy() ?

> +static int acpi_i2c_match_adapter(struct device *dev, void *data)
> +{
> +   struct i2c_adapter *adapter = i2c_verify_adapter(dev);
> +
> +   if (!adapter)
> +   return 0;
> +

> +   return ACPI_HANDLE(dev) == (acpi_handle)data;

What is suppose to return? Hidden bool to integer conversion is not
best option I suppose.

> +}
> +
> +static int acpi_i2c_match_device(struct device *dev, void *data)
> +{
> +   return ACPI_COMPANION(dev) == data;

Ditto.

> +}

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 1/2] clk: Add Oxford Semiconductor OXNAS Standard Clocks

[Stephen Boyd]

On 04/18, Neil Armstrong wrote:
> Add Oxford Semiconductor OXNAS SoC Family Standard Clocks support.
> 
> Signed-off-by: Neil Armstrong 
> ---

Applied to clk-next + the two line removal discussed.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v2 2/2] dt-bindings: Add Oxford Semiconductor OXNAS Standard Clocks bindings

[Stephen Boyd]

On 04/18, Neil Armstrong wrote:
> Acked-by: Rob Herring 
> Signed-off-by: Neil Armstrong 
> ---

Applied to clk-next

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v2 1/2] clk: Add Oxford Semiconductor OXNAS Standard Clocks

[Stephen Boyd]

On 04/18, Neil Armstrong wrote:
> Add Oxford Semiconductor OXNAS SoC Family Standard Clocks support.
> 
> Signed-off-by: Neil Armstrong 
> ---

Applied to clk-next + the two line removal discussed.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v2 2/2] dt-bindings: Add Oxford Semiconductor OXNAS Standard Clocks bindings

[Stephen Boyd]

On 04/18, Neil Armstrong wrote:
> Acked-by: Rob Herring 
> Signed-off-by: Neil Armstrong 
> ---

Applied to clk-next

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH] HID: uhid: Fixes a bug with userspace bluetooth stacks, which causes hangs during certain operations

[kbuild test robot]

Hi,

[auto build test WARNING on hid/for-next]
[also build test WARNING on v4.6-rc4 next-20160421]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improving the system]

url:
https://github.com/0day-ci/linux/commits/roderick-gaikai-com/HID-uhid-Fixes-a-bug-with-userspace-bluetooth-stacks-which-causes-hangs-during-certain-operations/20160422-050505
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jikos/hid.git for-next
config: x86_64-randconfig-x003-201616 (attached as .config)
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   In file included from arch/x86/include/asm/atomic.h:4:0,
from include/linux/atomic.h:4,
from drivers/hid/uhid.c:13:
   drivers/hid/uhid.c: In function 'uhid_event_from_user':
   drivers/hid/uhid.c:403:6: error: implicit declaration of function 
'is_compat_task' [-Werror=implicit-function-declaration]
 if (is_compat_task()) {
 ^
   include/linux/compiler.h:151:30: note: in definition of macro '__trace_if'
 if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
 ^
>> drivers/hid/uhid.c:403:2: note: in expansion of macro 'if'
 if (is_compat_task()) {
 ^
   cc1: some warnings being treated as errors

vim +/if +403 drivers/hid/uhid.c

   387  __u8 phys[64];
   388  __u8 uniq[64];
   389  
   390  compat_uptr_t rd_data;
   391  __u16 rd_size;
   392  
   393  __u16 bus;
   394  __u32 vendor;
   395  __u32 product;
   396  __u32 version;
   397  __u32 country;
   398  } __attribute__((__packed__));
   399  
   400  static int uhid_event_from_user(const char __user *buffer, size_t len,
   401  struct uhid_event *event)
   402  {
 > 403  if (is_compat_task()) {
   404  u32 type;
   405  
   406  if (get_user(type, buffer))
   407  return -EFAULT;
   408  
   409  if (type == UHID_CREATE) {
   410  /*
   411   * This is our messed up request with compat 
pointer.

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: Binary data

Re: [PATCH] HID: uhid: Fixes a bug with userspace bluetooth stacks, which causes hangs during certain operations

[kbuild test robot]

Hi,

[auto build test WARNING on hid/for-next]
[also build test WARNING on v4.6-rc4 next-20160421]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improving the system]

url:
https://github.com/0day-ci/linux/commits/roderick-gaikai-com/HID-uhid-Fixes-a-bug-with-userspace-bluetooth-stacks-which-causes-hangs-during-certain-operations/20160422-050505
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jikos/hid.git for-next
config: x86_64-randconfig-x003-201616 (attached as .config)
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   In file included from arch/x86/include/asm/atomic.h:4:0,
from include/linux/atomic.h:4,
from drivers/hid/uhid.c:13:
   drivers/hid/uhid.c: In function 'uhid_event_from_user':
   drivers/hid/uhid.c:403:6: error: implicit declaration of function 
'is_compat_task' [-Werror=implicit-function-declaration]
 if (is_compat_task()) {
 ^
   include/linux/compiler.h:151:30: note: in definition of macro '__trace_if'
 if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
 ^
>> drivers/hid/uhid.c:403:2: note: in expansion of macro 'if'
 if (is_compat_task()) {
 ^
   cc1: some warnings being treated as errors

vim +/if +403 drivers/hid/uhid.c

   387  __u8 phys[64];
   388  __u8 uniq[64];
   389  
   390  compat_uptr_t rd_data;
   391  __u16 rd_size;
   392  
   393  __u16 bus;
   394  __u32 vendor;
   395  __u32 product;
   396  __u32 version;
   397  __u32 country;
   398  } __attribute__((__packed__));
   399  
   400  static int uhid_event_from_user(const char __user *buffer, size_t len,
   401  struct uhid_event *event)
   402  {
 > 403  if (is_compat_task()) {
   404  u32 type;
   405  
   406  if (get_user(type, buffer))
   407  return -EFAULT;
   408  
   409  if (type == UHID_CREATE) {
   410  /*
   411   * This is our messed up request with compat 
pointer.

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: Binary data

Re: [PATCH] HID: uhid: Fixes a bug with userspace bluetooth stacks, which causes hangs during certain operations

[kbuild test robot]

Hi,

[auto build test ERROR on hid/for-next]
[also build test ERROR on v4.6-rc4 next-20160421]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improving the system]

url:
https://github.com/0day-ci/linux/commits/roderick-gaikai-com/HID-uhid-Fixes-a-bug-with-userspace-bluetooth-stacks-which-causes-hangs-during-certain-operations/20160422-050505
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jikos/hid.git for-next
config: x86_64-randconfig-x010-201616 (attached as .config)
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/hid/uhid.c: In function 'uhid_event_from_user':
>> drivers/hid/uhid.c:403:6: error: implicit declaration of function 
>> 'is_compat_task' [-Werror=implicit-function-declaration]
 if (is_compat_task()) {
 ^
   cc1: some warnings being treated as errors

vim +/is_compat_task +403 drivers/hid/uhid.c

   397  __u32 country;
   398  } __attribute__((__packed__));
   399  
   400  static int uhid_event_from_user(const char __user *buffer, size_t len,
   401  struct uhid_event *event)
   402  {
 > 403  if (is_compat_task()) {
   404  u32 type;
   405  
   406  if (get_user(type, buffer))

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: Binary data

Re: [PATCH] HID: uhid: Fixes a bug with userspace bluetooth stacks, which causes hangs during certain operations

[kbuild test robot]

Hi,

[auto build test ERROR on hid/for-next]
[also build test ERROR on v4.6-rc4 next-20160421]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improving the system]

url:
https://github.com/0day-ci/linux/commits/roderick-gaikai-com/HID-uhid-Fixes-a-bug-with-userspace-bluetooth-stacks-which-causes-hangs-during-certain-operations/20160422-050505
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jikos/hid.git for-next
config: x86_64-randconfig-x010-201616 (attached as .config)
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/hid/uhid.c: In function 'uhid_event_from_user':
>> drivers/hid/uhid.c:403:6: error: implicit declaration of function 
>> 'is_compat_task' [-Werror=implicit-function-declaration]
 if (is_compat_task()) {
 ^
   cc1: some warnings being treated as errors

vim +/is_compat_task +403 drivers/hid/uhid.c

   397  __u32 country;
   398  } __attribute__((__packed__));
   399  
   400  static int uhid_event_from_user(const char __user *buffer, size_t len,
   401  struct uhid_event *event)
   402  {
 > 403  if (is_compat_task()) {
   404  u32 type;
   405  
   406  if (get_user(type, buffer))

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: Binary data

Re: [PATCH] platform:x86 decouple telemetry driver from the optional IPC resources

[Andy Shevchenko]

On Fri, Apr 15, 2016 at 8:05 AM, Darren Hart  wrote:

>>  +   /*
>>  +* The following resources are optional
>>  +* - ISPDRIVER_IPC BASE_DATA
>>  +* - ISPDRIVER_IPC BASE_IFACE
>>  +* - GTDRIVER_IPC BASE_DATA
>>  +* - GTDRIVER_IPC BASE_IFACE
>>  +*/
>>  res = platform_get_resource(pdev, IORESOURCE_MEM, 2);
>>  -   addr = devm_ioremap_resource(>dev, res);
>>  -   if (IS_ERR(addr))
>>  -   return PTR_ERR(addr);
>>  -   punit_ipcdev->base[ISPDRIVER_IPC][BASE_DATA] = addr;
>>  +   if (res) {
>>  +   addr = devm_ioremap_resource(>dev, res);
>>  +   if (!IS_ERR(addr))
>>  +   punit_ipcdev->base[ISPDRIVER_IPC][BASE_DATA] = 
>>  addr;
>>  +   }
>> >>>
>> >>> And here, what about just replacing return to dev_warn()?
>> >>
>> >> I don't think we need to continue the subsequent ops if an error address
>> >> returns.
>> >
>> > Why is that? Will the driver fail to provide any functionality? Or could 
>> > it be
>> > the other IFACEs could still be of some use?
>> >
>> > This one does need a justification.
>> >
>> We discussed this.
>> - For the necessary resources, if we obtain an error address, we should
>> return immediately.
>> - For the optional resources, we keep quiet if we don't get them, that
>> is, not throwing a warning out.
>
> Andy, he's checking for "res" now too, which is a good extra check since
> devm_ioremap_resource will issue a dev_err "invalid resource" if it's NULL, 
> even
> though in our case, that's expected for an optional resource. This adds the
> extra nesting, and a dev_warn wouldn't be appropriate for an option resource.
>
> I'm happy to queue this to fixes at this point. Andy, are you OK with the
> resolution here?

Gmail hided this mail from me by some reason. I'm fine with the resolution.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] platform:x86 decouple telemetry driver from the optional IPC resources

[Andy Shevchenko]

On Fri, Apr 15, 2016 at 8:05 AM, Darren Hart  wrote:

>>  +   /*
>>  +* The following resources are optional
>>  +* - ISPDRIVER_IPC BASE_DATA
>>  +* - ISPDRIVER_IPC BASE_IFACE
>>  +* - GTDRIVER_IPC BASE_DATA
>>  +* - GTDRIVER_IPC BASE_IFACE
>>  +*/
>>  res = platform_get_resource(pdev, IORESOURCE_MEM, 2);
>>  -   addr = devm_ioremap_resource(>dev, res);
>>  -   if (IS_ERR(addr))
>>  -   return PTR_ERR(addr);
>>  -   punit_ipcdev->base[ISPDRIVER_IPC][BASE_DATA] = addr;
>>  +   if (res) {
>>  +   addr = devm_ioremap_resource(>dev, res);
>>  +   if (!IS_ERR(addr))
>>  +   punit_ipcdev->base[ISPDRIVER_IPC][BASE_DATA] = 
>>  addr;
>>  +   }
>> >>>
>> >>> And here, what about just replacing return to dev_warn()?
>> >>
>> >> I don't think we need to continue the subsequent ops if an error address
>> >> returns.
>> >
>> > Why is that? Will the driver fail to provide any functionality? Or could 
>> > it be
>> > the other IFACEs could still be of some use?
>> >
>> > This one does need a justification.
>> >
>> We discussed this.
>> - For the necessary resources, if we obtain an error address, we should
>> return immediately.
>> - For the optional resources, we keep quiet if we don't get them, that
>> is, not throwing a warning out.
>
> Andy, he's checking for "res" now too, which is a good extra check since
> devm_ioremap_resource will issue a dev_err "invalid resource" if it's NULL, 
> even
> though in our case, that's expected for an optional resource. This adds the
> extra nesting, and a dev_warn wouldn't be appropriate for an option resource.
>
> I'm happy to queue this to fixes at this point. Andy, are you OK with the
> resolution here?

Gmail hided this mail from me by some reason. I'm fine with the resolution.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] ARM64: dts: rockchip: add core dtsi file for RK3399 SoCs

[Marc Zyngier]

On Thu, 21 Apr 2016 22:24:09 +0200
Heiko Stübner  wrote:

> Am Donnerstag, 21. April 2016, 12:30:18 schrieb Marc Zyngier:
> > On Thu, 21 Apr 2016 18:47:20 +0800
> > 
> > "Huang, Tao"  wrote:
> > > Hi, Mark:
> > > 
> > > On 2016年04月21日 18:19, Mark Rutland wrote:
> > > > On Thu, Apr 21, 2016 at 11:58:12AM +0800, Jianqun Xu wrote:
> > > >> +  cpu_l0: cpu@0 {
> > > >> +  device_type = "cpu";
> > > >> +  compatible = "arm,cortex-a53", "arm,armv8";
> > > >> +  reg = <0x0 0x0>;
> > > >> +  enable-method = "psci";
> > > >> +  #cooling-cells = <2>; /* min followed by max */
> > > >> +  clocks = < ARMCLKL>;
> > > >> +  };
> > > >> +  cpu_b0: cpu@100 {
> > > >> +  device_type = "cpu";
> > > >> +  compatible = "arm,cortex-a72", "arm,armv8";
> > > >> +  reg = <0x0 0x100>;
> > > >> +  enable-method = "psci";
> > > >> +  #cooling-cells = <2>; /* min followed by max */
> > > >> +  clocks = < ARMCLKB>;
> > > >> +  };
> > > >> +
> > > >> +  arm-pmu {
> > > >> +  compatible = "arm,armv8-pmuv3";
> > > >> +  interrupts = ;
> > > >> +  };
> > > > 
> > > > This is wrong, and must go. There should be a separate node for the PMU
> > > > of each microarchitecture, with the appropriate compatible string to
> > > > represent that (see the juno dts).
> > > 
> > > You are right. The first version we wrote is:
> > > pmu_a53 {
> > > 
> > > compatible = "arm,cortex-a53-pmu";
> > > interrupts = ;
> > > interrupt-affinity = <_l0>,
> > > 
> > >  <_l1>,
> > >  <_l2>,
> > >  <_l3>;
> > > 
> > > };
> > > 
> > > pmu_a72 {
> > > 
> > > compatible = "arm,cortex-a72-pmu";
> > > interrupts = ;
> > > interrupt-affinity = <_b0>,
> > > 
> > >  <_b1>;
> > > 
> > > };
> > > 
> > > but unfortunately, the arm pmu driver do not support PPI in two cluster
> > > well,
> > > so we have to replace with this implementation.
> > > 
> > > > In this case things are messier as the same PPI number is being used
> > > > across clusters. Marc (Cc'd) has been working on PPI partitions, which
> > > > should allow us to support that.
> > > 
> > > Great! So what we can do right now? Wait this feature, and delete
> > > arm-pmu node?
> > 
> > I'd rather you have a look at the patches, test them with your HW,
> > and comment on what doesn't work!
> 
> I would think we could do it in two tracks, testing and fixing but also 
> letting 
> the rk3399 devicetrees move forward without the pmu at first :-) .

Where would the fun be then? ;-)

M.
-- 
Jazz is not dead. It just smells funny.

Re: [PATCH] ARM64: dts: rockchip: add core dtsi file for RK3399 SoCs

[Marc Zyngier]

On Thu, 21 Apr 2016 22:24:09 +0200
Heiko Stübner  wrote:

> Am Donnerstag, 21. April 2016, 12:30:18 schrieb Marc Zyngier:
> > On Thu, 21 Apr 2016 18:47:20 +0800
> > 
> > "Huang, Tao"  wrote:
> > > Hi, Mark:
> > > 
> > > On 2016年04月21日 18:19, Mark Rutland wrote:
> > > > On Thu, Apr 21, 2016 at 11:58:12AM +0800, Jianqun Xu wrote:
> > > >> +  cpu_l0: cpu@0 {
> > > >> +  device_type = "cpu";
> > > >> +  compatible = "arm,cortex-a53", "arm,armv8";
> > > >> +  reg = <0x0 0x0>;
> > > >> +  enable-method = "psci";
> > > >> +  #cooling-cells = <2>; /* min followed by max */
> > > >> +  clocks = < ARMCLKL>;
> > > >> +  };
> > > >> +  cpu_b0: cpu@100 {
> > > >> +  device_type = "cpu";
> > > >> +  compatible = "arm,cortex-a72", "arm,armv8";
> > > >> +  reg = <0x0 0x100>;
> > > >> +  enable-method = "psci";
> > > >> +  #cooling-cells = <2>; /* min followed by max */
> > > >> +  clocks = < ARMCLKB>;
> > > >> +  };
> > > >> +
> > > >> +  arm-pmu {
> > > >> +  compatible = "arm,armv8-pmuv3";
> > > >> +  interrupts = ;
> > > >> +  };
> > > > 
> > > > This is wrong, and must go. There should be a separate node for the PMU
> > > > of each microarchitecture, with the appropriate compatible string to
> > > > represent that (see the juno dts).
> > > 
> > > You are right. The first version we wrote is:
> > > pmu_a53 {
> > > 
> > > compatible = "arm,cortex-a53-pmu";
> > > interrupts = ;
> > > interrupt-affinity = <_l0>,
> > > 
> > >  <_l1>,
> > >  <_l2>,
> > >  <_l3>;
> > > 
> > > };
> > > 
> > > pmu_a72 {
> > > 
> > > compatible = "arm,cortex-a72-pmu";
> > > interrupts = ;
> > > interrupt-affinity = <_b0>,
> > > 
> > >  <_b1>;
> > > 
> > > };
> > > 
> > > but unfortunately, the arm pmu driver do not support PPI in two cluster
> > > well,
> > > so we have to replace with this implementation.
> > > 
> > > > In this case things are messier as the same PPI number is being used
> > > > across clusters. Marc (Cc'd) has been working on PPI partitions, which
> > > > should allow us to support that.
> > > 
> > > Great! So what we can do right now? Wait this feature, and delete
> > > arm-pmu node?
> > 
> > I'd rather you have a look at the patches, test them with your HW,
> > and comment on what doesn't work!
> 
> I would think we could do it in two tracks, testing and fixing but also 
> letting 
> the rk3399 devicetrees move forward without the pmu at first :-) .

Where would the fun be then? ;-)

M.
-- 
Jazz is not dead. It just smells funny.

Re: [PATCH] platform:x86 decouple telemetry driver from the optional IPC resources

[Andy Shevchenko]

On Fri, Apr 15, 2016 at 3:32 AM, Darren Hart  wrote:
> On Sun, Apr 10, 2016 at 09:46:48PM +0800, Li, Aubrey wrote:
>> On 2016/4/10 21:17, Andy Shevchenko wrote:
>> > On Thu, Mar 31, 2016 at 10:28 PM, Aubrey Li  
>> > wrote:
>> >> Currently the optional IPC resources prevent telemetry driver from
>> >> probing if these resources are not in ACPI table. This patch decouples
>> >> telemetry driver from these optional resources, so that telemetry driver
>> >> has dependency only on the necessary ACPI resources.
>> >
>> > Darren, I have comments as well.

>> >> --- a/drivers/platform/x86/intel_pmc_ipc.c
>> >> +++ b/drivers/platform/x86/intel_pmc_ipc.c
>> >> @@ -686,8 +686,8 @@ static int ipc_plat_get_res(struct platform_device 
>> >> *pdev)
>> >> ipcdev.acpi_io_size = size;
>> >> dev_info(>dev, "io res: %pR\n", res);
>> >>
>> >> -   /* This is index 0 to cover BIOS data register */
>> >> punit_res = punit_res_array;
>> >> +   /* This is index 0 to cover BIOS data register */
>> >> res = platform_get_resource(pdev, IORESOURCE_MEM,
>> >> PLAT_RESOURCE_BIOS_DATA_INDEX);
>> >> if (!res) {
>> >> @@ -697,55 +697,51 @@ static int ipc_plat_get_res(struct platform_device 
>> >> *pdev)
>> >> *punit_res = *res;
>> >> dev_info(>dev, "punit BIOS data res: %pR\n", res);
>> >>
>> >> +   /* This is index 1 to cover BIOS interface register */
>> >> res = platform_get_resource(pdev, IORESOURCE_MEM,
>> >> PLAT_RESOURCE_BIOS_IFACE_INDEX);
>> >> if (!res) {
>> >> dev_err(>dev, "Failed to get res of punit BIOS 
>> >> iface\n");
>> >> return -ENXIO;
>> >> }
>> >> -   /* This is index 1 to cover BIOS interface register */
>> >> *++punit_res = *res;
>> >> dev_info(>dev, "punit BIOS interface res: %pR\n", res);
>> >>
>> >> +   /* This is index 2 to cover ISP data register, optional */
>> >
>> > All above looks like a commentary fixes (except an additional
>> > 'optional' word in one case). Can you do this separately?
>>
>> I don't think it's necessary.
>>
>
> This is typically necessary as you would not want the comment fixes above to 
> be
> backed out if the functional changes below were found to be buggy and 
> reverted.
> This is why we encourage small functional changes. It protects against
> inadvertent reverts and facilitates review.
>
> That said, these comment changes continue below in a way that makes it a bit
> difficult to isolate them out, so I do not particularly object.
>
> That said, everyone should understand that Andy is part of the
> platform-driver-x86 maintainer team so please respect his comments as such.
>
>> >
>> >
>> >> res = platform_get_resource(pdev, IORESOURCE_MEM,
>> >> PLAT_RESOURCE_ISP_DATA_INDEX);
>> >> -   if (!res) {
>> >> -   dev_err(>dev, "Failed to get res of punit ISP 
>> >> data\n");
>> >> -   return -ENXIO;
>> >> +   ++punit_res;
>> >> +   if (res) {
>> >> +   *punit_res = *res;
>> >> +   dev_info(>dev, "punit ISP data res: %pR\n", res);
>> >
>> > Okay, what if you re-arrange this to some helper first
>> >
>>
>> Thanks for the idea, but I don't like a helper here, did you see
>> anything harmful of the current implementation?
>
> In both arguments, we need to identify the WHY.
>
> I imagine Andy is trying to reduce the copy and paste potential for error as
> well as error introduction in future patches. There are... 7 or so cases with
> near identical usage, that's a compelling argument for a refactor such as the
> helper Andy suggests.

Correct, But it might be done in a separate patch I suppose.

> Aubrey, you said you don't like it. Why is that? Will it not save enough lines
> of code to be worth it? Are you concerned about revalidating the change?
>
> In my opinion, a refactor is a good suggestion, but I would be OK with this
> patch as it is and a refactor to follow. I hesitate to do this when the 
> refactor
> is really critical as it may not happen, but in this case, it doesn't seem
> absolutely necessary.
>
>>
>> > int …_assign_res(*pdev, index, *punit_res)
>> > {
>> >  struct resource res;
>> >  res = platform_get_resource(pdev, …, index);
>> >  if (!res)
>> >   return -ERRNO;
>> >  *punit_res = *res;
>> >  dev_dbg(%pR);
>> >  return 0;
>> > }
>> >
>> > In this patch you move to optional by
>> > dev_err -> dev_warn
>> >
>> > and use
>> >
>> > if (ret)
>> >   dev_warn( "…skip  optional resource…" );
>> >
>> > instead of
>> > if (ret) {
>> >  dev_err();
>> >  return ret;
>> > }

>> >> @@ -239,29 +244,40 @@ static int intel_punit_get_bars(struct 
>> >> platform_device *pdev)
>> >> return PTR_ERR(addr);
>> >> punit_ipcdev->base[BIOS_IPC][BASE_IFACE] = addr;
>> >>
>> >> +   /*
>> >> +* The 

Re: [PATCH] platform:x86 decouple telemetry driver from the optional IPC resources

[Andy Shevchenko]

On Fri, Apr 15, 2016 at 3:32 AM, Darren Hart  wrote:
> On Sun, Apr 10, 2016 at 09:46:48PM +0800, Li, Aubrey wrote:
>> On 2016/4/10 21:17, Andy Shevchenko wrote:
>> > On Thu, Mar 31, 2016 at 10:28 PM, Aubrey Li  
>> > wrote:
>> >> Currently the optional IPC resources prevent telemetry driver from
>> >> probing if these resources are not in ACPI table. This patch decouples
>> >> telemetry driver from these optional resources, so that telemetry driver
>> >> has dependency only on the necessary ACPI resources.
>> >
>> > Darren, I have comments as well.

>> >> --- a/drivers/platform/x86/intel_pmc_ipc.c
>> >> +++ b/drivers/platform/x86/intel_pmc_ipc.c
>> >> @@ -686,8 +686,8 @@ static int ipc_plat_get_res(struct platform_device 
>> >> *pdev)
>> >> ipcdev.acpi_io_size = size;
>> >> dev_info(>dev, "io res: %pR\n", res);
>> >>
>> >> -   /* This is index 0 to cover BIOS data register */
>> >> punit_res = punit_res_array;
>> >> +   /* This is index 0 to cover BIOS data register */
>> >> res = platform_get_resource(pdev, IORESOURCE_MEM,
>> >> PLAT_RESOURCE_BIOS_DATA_INDEX);
>> >> if (!res) {
>> >> @@ -697,55 +697,51 @@ static int ipc_plat_get_res(struct platform_device 
>> >> *pdev)
>> >> *punit_res = *res;
>> >> dev_info(>dev, "punit BIOS data res: %pR\n", res);
>> >>
>> >> +   /* This is index 1 to cover BIOS interface register */
>> >> res = platform_get_resource(pdev, IORESOURCE_MEM,
>> >> PLAT_RESOURCE_BIOS_IFACE_INDEX);
>> >> if (!res) {
>> >> dev_err(>dev, "Failed to get res of punit BIOS 
>> >> iface\n");
>> >> return -ENXIO;
>> >> }
>> >> -   /* This is index 1 to cover BIOS interface register */
>> >> *++punit_res = *res;
>> >> dev_info(>dev, "punit BIOS interface res: %pR\n", res);
>> >>
>> >> +   /* This is index 2 to cover ISP data register, optional */
>> >
>> > All above looks like a commentary fixes (except an additional
>> > 'optional' word in one case). Can you do this separately?
>>
>> I don't think it's necessary.
>>
>
> This is typically necessary as you would not want the comment fixes above to 
> be
> backed out if the functional changes below were found to be buggy and 
> reverted.
> This is why we encourage small functional changes. It protects against
> inadvertent reverts and facilitates review.
>
> That said, these comment changes continue below in a way that makes it a bit
> difficult to isolate them out, so I do not particularly object.
>
> That said, everyone should understand that Andy is part of the
> platform-driver-x86 maintainer team so please respect his comments as such.
>
>> >
>> >
>> >> res = platform_get_resource(pdev, IORESOURCE_MEM,
>> >> PLAT_RESOURCE_ISP_DATA_INDEX);
>> >> -   if (!res) {
>> >> -   dev_err(>dev, "Failed to get res of punit ISP 
>> >> data\n");
>> >> -   return -ENXIO;
>> >> +   ++punit_res;
>> >> +   if (res) {
>> >> +   *punit_res = *res;
>> >> +   dev_info(>dev, "punit ISP data res: %pR\n", res);
>> >
>> > Okay, what if you re-arrange this to some helper first
>> >
>>
>> Thanks for the idea, but I don't like a helper here, did you see
>> anything harmful of the current implementation?
>
> In both arguments, we need to identify the WHY.
>
> I imagine Andy is trying to reduce the copy and paste potential for error as
> well as error introduction in future patches. There are... 7 or so cases with
> near identical usage, that's a compelling argument for a refactor such as the
> helper Andy suggests.

Correct, But it might be done in a separate patch I suppose.

> Aubrey, you said you don't like it. Why is that? Will it not save enough lines
> of code to be worth it? Are you concerned about revalidating the change?
>
> In my opinion, a refactor is a good suggestion, but I would be OK with this
> patch as it is and a refactor to follow. I hesitate to do this when the 
> refactor
> is really critical as it may not happen, but in this case, it doesn't seem
> absolutely necessary.
>
>>
>> > int …_assign_res(*pdev, index, *punit_res)
>> > {
>> >  struct resource res;
>> >  res = platform_get_resource(pdev, …, index);
>> >  if (!res)
>> >   return -ERRNO;
>> >  *punit_res = *res;
>> >  dev_dbg(%pR);
>> >  return 0;
>> > }
>> >
>> > In this patch you move to optional by
>> > dev_err -> dev_warn
>> >
>> > and use
>> >
>> > if (ret)
>> >   dev_warn( "…skip  optional resource…" );
>> >
>> > instead of
>> > if (ret) {
>> >  dev_err();
>> >  return ret;
>> > }

>> >> @@ -239,29 +244,40 @@ static int intel_punit_get_bars(struct 
>> >> platform_device *pdev)
>> >> return PTR_ERR(addr);
>> >> punit_ipcdev->base[BIOS_IPC][BASE_IFACE] = addr;
>> >>
>> >> +   /*
>> >> +* The following resources are optional
>> >> +* 

Re: [PATCH] wcn36xx: Set SMD timeout to 10 seconds

[John Stultz]

On Thu, Apr 21, 2016 at 2:09 PM, Bjorn Andersson
 wrote:
> After booting the wireless subsystem and uploading the NV blob to the
> WCNSS_CTRL service the remote continues to do things and will not start
> servicing wlan-requests for another 2-5 seconds (measured).
>
> The downstream code does not have any special handling for this case,
> but has a timeout of 10 seconds for the communication layer. By
> extending the wcn36xx timeout to match this we follows the same flow for
> the boot procedure and can successfully configure WiFi as wlan0 is
> registered.
>
> Signed-off-by: Bjorn Andersson 

I've been using this with my nexus7 tree, and its avoided issues I was
seeing without it.

Tested-by: John Stultz 

thanks
-john

Re: [PATCH] wcn36xx: Set SMD timeout to 10 seconds

[John Stultz]

On Thu, Apr 21, 2016 at 2:09 PM, Bjorn Andersson
 wrote:
> After booting the wireless subsystem and uploading the NV blob to the
> WCNSS_CTRL service the remote continues to do things and will not start
> servicing wlan-requests for another 2-5 seconds (measured).
>
> The downstream code does not have any special handling for this case,
> but has a timeout of 10 seconds for the communication layer. By
> extending the wcn36xx timeout to match this we follows the same flow for
> the boot procedure and can successfully configure WiFi as wlan0 is
> registered.
>
> Signed-off-by: Bjorn Andersson 

I've been using this with my nexus7 tree, and its avoided issues I was
seeing without it.

Tested-by: John Stultz 

thanks
-john

Re: [kbuild-all] mipsel-linux-gnu-gcc: error: unrecognized command line option '-mcompact-branches=optimal'

[Maciej W. Rozycki]

On Thu, 21 Apr 2016, Ralf Baechle wrote:

> >  I don't think it makes sense as the compiler won't support MIPSr6 code 
> > anyway, so first it'll bail out on `-march=mips32r6', and if we go even 
> > further and disable that too, then GAS will probably break somewhere on 
> > inline asm and GCC will produce code which does not make sense otherwise.
> 
> GCC 5.2.0 claims to support mips32r6 and mips64r6.  It's just the option
> -mcompact-branches which seem to have been added later only.

 Ah, I see -- I didn't track the timeline of support for this compiler's 
option and I took it from an earlier response that the compiler does not 
support R6 at all.

 In that case however it looks to me like these `-mcompact-branches=' 
options (all the three we support) need to be wrapped into `$(call 
cc-option,...)'.  They do not affect any functionality and they are an 
optimisation choice only anyway (and therefore I wonder why they've been 
placed in arch/mips/Kconfig.debug rather than arch/mips/Kconfig).

  Maciej

Re: [kbuild-all] mipsel-linux-gnu-gcc: error: unrecognized command line option '-mcompact-branches=optimal'

[Maciej W. Rozycki]

On Thu, 21 Apr 2016, Ralf Baechle wrote:

> >  I don't think it makes sense as the compiler won't support MIPSr6 code 
> > anyway, so first it'll bail out on `-march=mips32r6', and if we go even 
> > further and disable that too, then GAS will probably break somewhere on 
> > inline asm and GCC will produce code which does not make sense otherwise.
> 
> GCC 5.2.0 claims to support mips32r6 and mips64r6.  It's just the option
> -mcompact-branches which seem to have been added later only.

 Ah, I see -- I didn't track the timeline of support for this compiler's 
option and I took it from an earlier response that the compiler does not 
support R6 at all.

 In that case however it looks to me like these `-mcompact-branches=' 
options (all the three we support) need to be wrapped into `$(call 
cc-option,...)'.  They do not affect any functionality and they are an 
optimisation choice only anyway (and therefore I wonder why they've been 
placed in arch/mips/Kconfig.debug rather than arch/mips/Kconfig).

  Maciej

[PATCH] wcn36xx: Set SMD timeout to 10 seconds

[Bjorn Andersson]

After booting the wireless subsystem and uploading the NV blob to the
WCNSS_CTRL service the remote continues to do things and will not start
servicing wlan-requests for another 2-5 seconds (measured).

The downstream code does not have any special handling for this case,
but has a timeout of 10 seconds for the communication layer. By
extending the wcn36xx timeout to match this we follows the same flow for
the boot procedure and can successfully configure WiFi as wlan0 is
registered.

Signed-off-by: Bjorn Andersson 
---
 drivers/net/wireless/ath/wcn36xx/smd.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/wcn36xx/smd.h 
b/drivers/net/wireless/ath/wcn36xx/smd.h
index e6aadd273c46..6310560901f0 100644
--- a/drivers/net/wireless/ath/wcn36xx/smd.h
+++ b/drivers/net/wireless/ath/wcn36xx/smd.h
@@ -24,7 +24,7 @@
 
 #define WCN36XX_HAL_BUF_SIZE   4096
 
-#define HAL_MSG_TIMEOUT 500
+#define HAL_MSG_TIMEOUT 1
 #define WCN36XX_SMSM_WLAN_TX_ENABLE0x0400
 #define WCN36XX_SMSM_WLAN_TX_RINGS_EMPTY   0x0200
 /* The PNO version info be contained in the rsp msg */
-- 
2.5.0

[PATCH] wcn36xx: Set SMD timeout to 10 seconds

[Bjorn Andersson]

After booting the wireless subsystem and uploading the NV blob to the
WCNSS_CTRL service the remote continues to do things and will not start
servicing wlan-requests for another 2-5 seconds (measured).

The downstream code does not have any special handling for this case,
but has a timeout of 10 seconds for the communication layer. By
extending the wcn36xx timeout to match this we follows the same flow for
the boot procedure and can successfully configure WiFi as wlan0 is
registered.

Signed-off-by: Bjorn Andersson 
---
 drivers/net/wireless/ath/wcn36xx/smd.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/wcn36xx/smd.h 
b/drivers/net/wireless/ath/wcn36xx/smd.h
index e6aadd273c46..6310560901f0 100644
--- a/drivers/net/wireless/ath/wcn36xx/smd.h
+++ b/drivers/net/wireless/ath/wcn36xx/smd.h
@@ -24,7 +24,7 @@
 
 #define WCN36XX_HAL_BUF_SIZE   4096
 
-#define HAL_MSG_TIMEOUT 500
+#define HAL_MSG_TIMEOUT 1
 #define WCN36XX_SMSM_WLAN_TX_ENABLE0x0400
 #define WCN36XX_SMSM_WLAN_TX_RINGS_EMPTY   0x0200
 /* The PNO version info be contained in the rsp msg */
-- 
2.5.0

Re: [PATCH v2] PM / Runtime: Only force-resume device if it has been force-suspended

[Laurent Pinchart]

Hi Rafael,

On Thursday 21 Apr 2016 23:02:06 Rafael J. Wysocki wrote:
> On Thu, Apr 21, 2016 at 10:57 PM, Laurent Pinchart wrote:
> > On Thursday 21 Apr 2016 21:52:56 Rafael J. Wysocki wrote:
> >> On Thursday, April 21, 2016 02:52:55 AM Laurent Pinchart wrote:
> >>> The pm_runtime_force_suspend() and pm_runtime_force_resume() helpers
> >>> are designed to help driver being RPM-centric by offering an easy way to
> >>> manage runtime PM state during system suspend and resume. The first
> >>> function will force the device into runtime suspend at system suspend
> >>> time, while the second one will perform the reverse operation at system
> >>> resume time.
> >>> 
> >>> However, the pm_runtime_force_resume() really forces resume, regardless
> >>> of whether the device was running or already suspended before the call
> >>> to pm_runtime_force_suspend(). This results in devices being runtime
> >>> resumed at system resume time when they shouldn't.
> >>> 
> >>> Fix this by recording whether the device has been forcefully suspended
> >>> in pm_runtime_force_suspend() and condition resume in
> >>> pm_runtime_force_resume() to that state.
> >>> 
> >>> All current users of pm_runtime_force_resume() call the function
> >>> unconditionally in their system resume handler (some actually set it as
> >>> the resume handler), all after calling pm_runtime_force_suspend() at
> >>> system suspend time. The change in behaviour should thus be safe.
> >>> 
> >>> Signed-off-by: Laurent Pinchart
> >>> 
> >>> Reviewed-by: Kevin Hilman 
> >> 
> >> Ulf, any comments?
> > 
> > Ulf has proposed a different approach in "[PATCH] PM / Runtime: Defer
> > resuming of the device in pm_runtime_force_resume()". I agree that using
> > usage_count is better than introducing a new state flag in struct
> > dev_pm_info, with a caveat: it doesn't work properly :-). We would have
> > to fix genpd first, as commented in a reply to Ulf's patch.
> 
> OK, thanks!
> 
> Since I'd prefer to avoid adding more state flags too, I'll let you
> guys noodle around this for a while more. :-)

Let's see what we can do in a reasonable time frame. We could decide to merge 
this patch as a temporary fix until the genpd rework is complete.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2] PM / Runtime: Only force-resume device if it has been force-suspended

[Laurent Pinchart]

Hi Rafael,

On Thursday 21 Apr 2016 23:02:06 Rafael J. Wysocki wrote:
> On Thu, Apr 21, 2016 at 10:57 PM, Laurent Pinchart wrote:
> > On Thursday 21 Apr 2016 21:52:56 Rafael J. Wysocki wrote:
> >> On Thursday, April 21, 2016 02:52:55 AM Laurent Pinchart wrote:
> >>> The pm_runtime_force_suspend() and pm_runtime_force_resume() helpers
> >>> are designed to help driver being RPM-centric by offering an easy way to
> >>> manage runtime PM state during system suspend and resume. The first
> >>> function will force the device into runtime suspend at system suspend
> >>> time, while the second one will perform the reverse operation at system
> >>> resume time.
> >>> 
> >>> However, the pm_runtime_force_resume() really forces resume, regardless
> >>> of whether the device was running or already suspended before the call
> >>> to pm_runtime_force_suspend(). This results in devices being runtime
> >>> resumed at system resume time when they shouldn't.
> >>> 
> >>> Fix this by recording whether the device has been forcefully suspended
> >>> in pm_runtime_force_suspend() and condition resume in
> >>> pm_runtime_force_resume() to that state.
> >>> 
> >>> All current users of pm_runtime_force_resume() call the function
> >>> unconditionally in their system resume handler (some actually set it as
> >>> the resume handler), all after calling pm_runtime_force_suspend() at
> >>> system suspend time. The change in behaviour should thus be safe.
> >>> 
> >>> Signed-off-by: Laurent Pinchart
> >>> 
> >>> Reviewed-by: Kevin Hilman 
> >> 
> >> Ulf, any comments?
> > 
> > Ulf has proposed a different approach in "[PATCH] PM / Runtime: Defer
> > resuming of the device in pm_runtime_force_resume()". I agree that using
> > usage_count is better than introducing a new state flag in struct
> > dev_pm_info, with a caveat: it doesn't work properly :-). We would have
> > to fix genpd first, as commented in a reply to Ulf's patch.
> 
> OK, thanks!
> 
> Since I'd prefer to avoid adding more state flags too, I'll let you
> guys noodle around this for a while more. :-)

Let's see what we can do in a reasonable time frame. We could decide to merge 
this patch as a temporary fix until the genpd rework is complete.

-- 
Regards,

Laurent Pinchart

Re: [PATCH] ARM64: dts: rockchip: add core dtsi file for RK3399 SoCs

[Rob Herring]

On Wed, Apr 20, 2016 at 10:58 PM, Jianqun Xu  wrote:
> This patch adds rk3399.dtsi for rk3399 found on Rockchip
> RK3399 SoCs, also add rk3399-evb.dts for Rockchip RK3399
> Evaluation Board.
>
> Patch is tested on RK3399 evb.
>
> Signed-off-by: Jianqun Xu 
> ---
>  arch/arm64/boot/dts/rockchip/Makefile   |1 +
>  arch/arm64/boot/dts/rockchip/rk3399-evb.dts |  537 
>  arch/arm64/boot/dts/rockchip/rk3399.dtsi| 1757 
> +++
>  3 files changed, 2295 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/rockchip/rk3399-evb.dts
>  create mode 100644 arch/arm64/boot/dts/rockchip/rk3399.dtsi
>
> diff --git a/arch/arm64/boot/dts/rockchip/Makefile 
> b/arch/arm64/boot/dts/rockchip/Makefile
> index df37865..7037a16 100644
> --- a/arch/arm64/boot/dts/rockchip/Makefile
> +++ b/arch/arm64/boot/dts/rockchip/Makefile
> @@ -1,6 +1,7 @@
>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3368-evb-act8846.dtb
>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3368-geekbox.dtb
>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3368-r88.dtb
> +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-evb.dtb
>
>  always := $(dtb-y)
>  subdir-y   := $(dts-dirs)
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-evb.dts 
> b/arch/arm64/boot/dts/rockchip/rk3399-evb.dts
> new file mode 100644
> index 000..4cb0028
> --- /dev/null
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-evb.dts
> @@ -0,0 +1,537 @@
> +/*
> + * Copyright (c) 2016 Fuzhou Rockchip Electronics Co., Ltd
> + *
> + * This file is dual-licensed: you can use it either under the terms
> + * of the GPL or the X11 license, at your option. Note that this dual
> + * licensing only applies to this file, and not this project as a
> + * whole.
> + *
> + *  a) This file is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of the
> + * License, or (at your option) any later version.
> + *
> + * This file is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * Or, alternatively,
> + *
> + *  b) Permission is hereby granted, free of charge, to any person
> + * obtaining a copy of this software and associated documentation
> + * files (the "Software"), to deal in the Software without
> + * restriction, including without limitation the rights to use,
> + * copy, modify, merge, publish, distribute, sublicense, and/or
> + * sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following
> + * conditions:
> + *
> + * The above copyright notice and this permission notice shall be
> + * included in all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
> + * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
> + * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
> + * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + * OTHER DEALINGS IN THE SOFTWARE.
> + */
> +
> +/dts-v1/;
> +
> +#include 
> +#include "rk3399.dtsi"
> +
> +/ {
> +   model = "Rockchip RK3399 Evaluation Board";
> +   compatible = "rockchip,evb", "rockchip,rk3399-evb";
> +
> +   chosen {
> +   bootargs = "console=uart,mmio32,0xff1a";
> +   };
> +
> +   ramoops_mem: ramoops_mem {
> +   reg = <0x0 0x10 0x0 0x10>;
> +   reg-names = "ramoops_mem";
> +   };
> +
> +   ramoops {
> +   compatible = "ramoops";
> +   record-size = <0x0 0x2>;
> +   console-size = <0x0 0x8>;
> +   ftrace-size = <0x0 0x1>;
> +   pmsg-size = <0x0 0x5>;
> +   memory-region = <_mem>;
> +   };

This binding is not upstream yet. It would be great if it was if you
want to pick up the last try.

Rob

Re: [PATCH] ARM64: dts: rockchip: add core dtsi file for RK3399 SoCs

[Rob Herring]

On Wed, Apr 20, 2016 at 10:58 PM, Jianqun Xu  wrote:
> This patch adds rk3399.dtsi for rk3399 found on Rockchip
> RK3399 SoCs, also add rk3399-evb.dts for Rockchip RK3399
> Evaluation Board.
>
> Patch is tested on RK3399 evb.
>
> Signed-off-by: Jianqun Xu 
> ---
>  arch/arm64/boot/dts/rockchip/Makefile   |1 +
>  arch/arm64/boot/dts/rockchip/rk3399-evb.dts |  537 
>  arch/arm64/boot/dts/rockchip/rk3399.dtsi| 1757 
> +++
>  3 files changed, 2295 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/rockchip/rk3399-evb.dts
>  create mode 100644 arch/arm64/boot/dts/rockchip/rk3399.dtsi
>
> diff --git a/arch/arm64/boot/dts/rockchip/Makefile 
> b/arch/arm64/boot/dts/rockchip/Makefile
> index df37865..7037a16 100644
> --- a/arch/arm64/boot/dts/rockchip/Makefile
> +++ b/arch/arm64/boot/dts/rockchip/Makefile
> @@ -1,6 +1,7 @@
>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3368-evb-act8846.dtb
>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3368-geekbox.dtb
>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3368-r88.dtb
> +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-evb.dtb
>
>  always := $(dtb-y)
>  subdir-y   := $(dts-dirs)
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-evb.dts 
> b/arch/arm64/boot/dts/rockchip/rk3399-evb.dts
> new file mode 100644
> index 000..4cb0028
> --- /dev/null
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-evb.dts
> @@ -0,0 +1,537 @@
> +/*
> + * Copyright (c) 2016 Fuzhou Rockchip Electronics Co., Ltd
> + *
> + * This file is dual-licensed: you can use it either under the terms
> + * of the GPL or the X11 license, at your option. Note that this dual
> + * licensing only applies to this file, and not this project as a
> + * whole.
> + *
> + *  a) This file is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of the
> + * License, or (at your option) any later version.
> + *
> + * This file is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * Or, alternatively,
> + *
> + *  b) Permission is hereby granted, free of charge, to any person
> + * obtaining a copy of this software and associated documentation
> + * files (the "Software"), to deal in the Software without
> + * restriction, including without limitation the rights to use,
> + * copy, modify, merge, publish, distribute, sublicense, and/or
> + * sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following
> + * conditions:
> + *
> + * The above copyright notice and this permission notice shall be
> + * included in all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
> + * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
> + * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
> + * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + * OTHER DEALINGS IN THE SOFTWARE.
> + */
> +
> +/dts-v1/;
> +
> +#include 
> +#include "rk3399.dtsi"
> +
> +/ {
> +   model = "Rockchip RK3399 Evaluation Board";
> +   compatible = "rockchip,evb", "rockchip,rk3399-evb";
> +
> +   chosen {
> +   bootargs = "console=uart,mmio32,0xff1a";
> +   };
> +
> +   ramoops_mem: ramoops_mem {
> +   reg = <0x0 0x10 0x0 0x10>;
> +   reg-names = "ramoops_mem";
> +   };
> +
> +   ramoops {
> +   compatible = "ramoops";
> +   record-size = <0x0 0x2>;
> +   console-size = <0x0 0x8>;
> +   ftrace-size = <0x0 0x1>;
> +   pmsg-size = <0x0 0x5>;
> +   memory-region = <_mem>;
> +   };

This binding is not upstream yet. It would be great if it was if you
want to pick up the last try.

Rob

[PATCH] HID: uhid: Fixes a bug with userspace bluetooth stacks, which causes hangs during certain operations

[roderick]

From: Roderick Colenbrander 

Many devices use userspace bluetooth stacks like BlueZ or Bluedroid in 
combination
with uhid. If any of these stacks is used with a HID device for which the driver
performs a HID request as part .probe (or technically another HID operation),
this results in a deadlock situation. The deadlock results in a 5 second timeout
for I/O operations in HID drivers, so isn't fatal, but none of the I/O 
operations
have a chance of succeeding.

The root cause for the problem is that uhid only allows for one request to be
processed at a time per uhid instance and locks out other operations. This means
that if a user space is creating a new HID device through 'UHID_CREATE', which
ultimately triggers '.probe' through the HID layer. Then any HID request e.g. a
read for calibration data would trigger a HID operation on uhid again, but it
won't go out to userspace, because it is still stuck in UHID_CREATE.
In addition bluetooth stacks are typically single threaded, so they wouldn't be
able to handle any requests while waiting on uhid.

Lucikly the UHID spec is somewhat flexible and allows for fixing the issue,
without breaking user space. The idea which the patch implements as discussed
with David Herrmann is to decouple adding of a hid device (which triggers 
.probe)
from UHID_CREATE. The work will kick off roughly once UHID_CREATE completed (or
else will wait a tiny bit of time in .probe for a lock). A HID driver has to 
call
HID to call 'hid_hw_start()' as part of .probe once it is ready for I/O, which
triggers UHID_START to user space. Any HID operations should function now within
.probe and won't deadlock because userspace is stuck on UHID_CREATE.

We verified this patch on Bluedroid with Android 6.0 and on desktop Linux with
BlueZ stacks. Prior to the patch they had the deadlock issue.

Signed-off-by: Roderick Colenbrander 
Cc: sta...@vger.kernel.org
---
 drivers/hid/uhid.c | 35 +--
 1 file changed, 25 insertions(+), 10 deletions(-)

diff --git a/drivers/hid/uhid.c b/drivers/hid/uhid.c
index 16b6f11..1a2032c 100644
--- a/drivers/hid/uhid.c
+++ b/drivers/hid/uhid.c
@@ -51,10 +51,26 @@ struct uhid_device {
u32 report_id;
u32 report_type;
struct uhid_event report_buf;
+   struct work_struct worker;
 };
 
 static struct miscdevice uhid_misc;
 
+static void uhid_device_add_worker(struct work_struct *work)
+{
+   struct uhid_device *uhid = container_of(work, struct uhid_device, 
worker);
+   int ret;
+
+   ret = hid_add_device(uhid->hid);
+   if (ret) {
+   hid_err(uhid->hid, "Cannot register HID device: error %d\n", 
ret);
+
+   hid_destroy_device(uhid->hid);
+   uhid->hid = NULL;
+   uhid->running = false;
+   }
+}
+
 static void uhid_queue(struct uhid_device *uhid, struct uhid_event *ev)
 {
__u8 newhead;
@@ -384,7 +400,7 @@ struct uhid_create_req_compat {
 static int uhid_event_from_user(const char __user *buffer, size_t len,
struct uhid_event *event)
 {
-   if (in_compat_syscall()) {
+   if (is_compat_task()) {
u32 type;
 
if (get_user(type, buffer))
@@ -498,18 +514,14 @@ static int uhid_dev_create2(struct uhid_device *uhid,
uhid->hid = hid;
uhid->running = true;
 
-   ret = hid_add_device(hid);
-   if (ret) {
-   hid_err(hid, "Cannot register HID device\n");
-   goto err_hid;
-   }
+   /* Adding of a HID device is done through a worker, to allow HID drivers
+* which use feature requests during .probe to work, without they would
+* be blocked on devlock, which is held by uhid_char_write.
+*/
+   schedule_work(>worker);
 
return 0;
 
-err_hid:
-   hid_destroy_device(hid);
-   uhid->hid = NULL;
-   uhid->running = false;
 err_free:
kfree(uhid->rd_data);
uhid->rd_data = NULL;
@@ -550,6 +562,8 @@ static int uhid_dev_destroy(struct uhid_device *uhid)
uhid->running = false;
wake_up_interruptible(>report_wait);
 
+   cancel_work_sync(>worker);
+
hid_destroy_device(uhid->hid);
kfree(uhid->rd_data);
 
@@ -612,6 +626,7 @@ static int uhid_char_open(struct inode *inode, struct file 
*file)
init_waitqueue_head(>waitq);
init_waitqueue_head(>report_wait);
uhid->running = false;
+   INIT_WORK(>worker, uhid_device_add_worker);
 
file->private_data = uhid;
nonseekable_open(inode, file);
-- 
2.5.5

[PATCH] HID: uhid: Fixes a bug with userspace bluetooth stacks, which causes hangs during certain operations

[roderick]

From: Roderick Colenbrander 

Many devices use userspace bluetooth stacks like BlueZ or Bluedroid in 
combination
with uhid. If any of these stacks is used with a HID device for which the driver
performs a HID request as part .probe (or technically another HID operation),
this results in a deadlock situation. The deadlock results in a 5 second timeout
for I/O operations in HID drivers, so isn't fatal, but none of the I/O 
operations
have a chance of succeeding.

The root cause for the problem is that uhid only allows for one request to be
processed at a time per uhid instance and locks out other operations. This means
that if a user space is creating a new HID device through 'UHID_CREATE', which
ultimately triggers '.probe' through the HID layer. Then any HID request e.g. a
read for calibration data would trigger a HID operation on uhid again, but it
won't go out to userspace, because it is still stuck in UHID_CREATE.
In addition bluetooth stacks are typically single threaded, so they wouldn't be
able to handle any requests while waiting on uhid.

Lucikly the UHID spec is somewhat flexible and allows for fixing the issue,
without breaking user space. The idea which the patch implements as discussed
with David Herrmann is to decouple adding of a hid device (which triggers 
.probe)
from UHID_CREATE. The work will kick off roughly once UHID_CREATE completed (or
else will wait a tiny bit of time in .probe for a lock). A HID driver has to 
call
HID to call 'hid_hw_start()' as part of .probe once it is ready for I/O, which
triggers UHID_START to user space. Any HID operations should function now within
.probe and won't deadlock because userspace is stuck on UHID_CREATE.

We verified this patch on Bluedroid with Android 6.0 and on desktop Linux with
BlueZ stacks. Prior to the patch they had the deadlock issue.

Signed-off-by: Roderick Colenbrander 
Cc: sta...@vger.kernel.org
---
 drivers/hid/uhid.c | 35 +--
 1 file changed, 25 insertions(+), 10 deletions(-)

diff --git a/drivers/hid/uhid.c b/drivers/hid/uhid.c
index 16b6f11..1a2032c 100644
--- a/drivers/hid/uhid.c
+++ b/drivers/hid/uhid.c
@@ -51,10 +51,26 @@ struct uhid_device {
u32 report_id;
u32 report_type;
struct uhid_event report_buf;
+   struct work_struct worker;
 };
 
 static struct miscdevice uhid_misc;
 
+static void uhid_device_add_worker(struct work_struct *work)
+{
+   struct uhid_device *uhid = container_of(work, struct uhid_device, 
worker);
+   int ret;
+
+   ret = hid_add_device(uhid->hid);
+   if (ret) {
+   hid_err(uhid->hid, "Cannot register HID device: error %d\n", 
ret);
+
+   hid_destroy_device(uhid->hid);
+   uhid->hid = NULL;
+   uhid->running = false;
+   }
+}
+
 static void uhid_queue(struct uhid_device *uhid, struct uhid_event *ev)
 {
__u8 newhead;
@@ -384,7 +400,7 @@ struct uhid_create_req_compat {
 static int uhid_event_from_user(const char __user *buffer, size_t len,
struct uhid_event *event)
 {
-   if (in_compat_syscall()) {
+   if (is_compat_task()) {
u32 type;
 
if (get_user(type, buffer))
@@ -498,18 +514,14 @@ static int uhid_dev_create2(struct uhid_device *uhid,
uhid->hid = hid;
uhid->running = true;
 
-   ret = hid_add_device(hid);
-   if (ret) {
-   hid_err(hid, "Cannot register HID device\n");
-   goto err_hid;
-   }
+   /* Adding of a HID device is done through a worker, to allow HID drivers
+* which use feature requests during .probe to work, without they would
+* be blocked on devlock, which is held by uhid_char_write.
+*/
+   schedule_work(>worker);
 
return 0;
 
-err_hid:
-   hid_destroy_device(hid);
-   uhid->hid = NULL;
-   uhid->running = false;
 err_free:
kfree(uhid->rd_data);
uhid->rd_data = NULL;
@@ -550,6 +562,8 @@ static int uhid_dev_destroy(struct uhid_device *uhid)
uhid->running = false;
wake_up_interruptible(>report_wait);
 
+   cancel_work_sync(>worker);
+
hid_destroy_device(uhid->hid);
kfree(uhid->rd_data);
 
@@ -612,6 +626,7 @@ static int uhid_char_open(struct inode *inode, struct file 
*file)
init_waitqueue_head(>waitq);
init_waitqueue_head(>report_wait);
uhid->running = false;
+   INIT_WORK(>worker, uhid_device_add_worker);
 
file->private_data = uhid;
nonseekable_open(inode, file);
-- 
2.5.5

Re: [PATCH v2] PM / Runtime: Only force-resume device if it has been force-suspended

[Rafael J. Wysocki]

On Thu, Apr 21, 2016 at 10:57 PM, Laurent Pinchart
 wrote:
> Hi Rafael,
>
> On Thursday 21 Apr 2016 21:52:56 Rafael J. Wysocki wrote:
>> On Thursday, April 21, 2016 02:52:55 AM Laurent Pinchart wrote:
>> > The pm_runtime_force_suspend() and pm_runtime_force_resume() helpers are
>> > designed to help driver being RPM-centric by offering an easy way to
>> > manage runtime PM state during system suspend and resume. The first
>> > function will force the device into runtime suspend at system suspend
>> > time, while the second one will perform the reverse operation at system
>> > resume time.
>> >
>> > However, the pm_runtime_force_resume() really forces resume, regardless
>> > of whether the device was running or already suspended before the call
>> > to pm_runtime_force_suspend(). This results in devices being runtime
>> > resumed at system resume time when they shouldn't.
>> >
>> > Fix this by recording whether the device has been forcefully suspended
>> > in pm_runtime_force_suspend() and condition resume in
>> > pm_runtime_force_resume() to that state.
>> >
>> > All current users of pm_runtime_force_resume() call the function
>> > unconditionally in their system resume handler (some actually set it as
>> > the resume handler), all after calling pm_runtime_force_suspend() at
>> > system suspend time. The change in behaviour should thus be safe.
>> >
>> > Signed-off-by: Laurent Pinchart
>> > 
>> > Reviewed-by: Kevin Hilman 
>>
>> Ulf, any comments?
>
> Ulf has proposed a different approach in "[PATCH] PM / Runtime: Defer resuming
> of the device in pm_runtime_force_resume()". I agree that using usage_count is
> better than introducing a new state flag in struct dev_pm_info, with a caveat:
> it doesn't work properly :-). We would have to fix genpd first, as commented
> in a reply to Ulf's patch.

OK, thanks!

Since I'd prefer to avoid adding more state flags too, I'll let you
guys noodle around this for a while more. :-)

Re: [PATCH v2] PM / Runtime: Only force-resume device if it has been force-suspended

[Rafael J. Wysocki]

On Thu, Apr 21, 2016 at 10:57 PM, Laurent Pinchart
 wrote:
> Hi Rafael,
>
> On Thursday 21 Apr 2016 21:52:56 Rafael J. Wysocki wrote:
>> On Thursday, April 21, 2016 02:52:55 AM Laurent Pinchart wrote:
>> > The pm_runtime_force_suspend() and pm_runtime_force_resume() helpers are
>> > designed to help driver being RPM-centric by offering an easy way to
>> > manage runtime PM state during system suspend and resume. The first
>> > function will force the device into runtime suspend at system suspend
>> > time, while the second one will perform the reverse operation at system
>> > resume time.
>> >
>> > However, the pm_runtime_force_resume() really forces resume, regardless
>> > of whether the device was running or already suspended before the call
>> > to pm_runtime_force_suspend(). This results in devices being runtime
>> > resumed at system resume time when they shouldn't.
>> >
>> > Fix this by recording whether the device has been forcefully suspended
>> > in pm_runtime_force_suspend() and condition resume in
>> > pm_runtime_force_resume() to that state.
>> >
>> > All current users of pm_runtime_force_resume() call the function
>> > unconditionally in their system resume handler (some actually set it as
>> > the resume handler), all after calling pm_runtime_force_suspend() at
>> > system suspend time. The change in behaviour should thus be safe.
>> >
>> > Signed-off-by: Laurent Pinchart
>> > 
>> > Reviewed-by: Kevin Hilman 
>>
>> Ulf, any comments?
>
> Ulf has proposed a different approach in "[PATCH] PM / Runtime: Defer resuming
> of the device in pm_runtime_force_resume()". I agree that using usage_count is
> better than introducing a new state flag in struct dev_pm_info, with a caveat:
> it doesn't work properly :-). We would have to fix genpd first, as commented
> in a reply to Ulf's patch.

OK, thanks!

Since I'd prefer to avoid adding more state flags too, I'll let you
guys noodle around this for a while more. :-)

Re: [PATCH] ixgbevf: Fix relaxed order settings in VF driver

[Alexander Duyck]

On Thu, Apr 21, 2016 at 12:39 PM, Babu Moger  wrote:
> Hi Alex,
>
> On 4/21/2016 2:22 PM, Alexander Duyck wrote:
>> On Thu, Apr 21, 2016 at 11:13 AM, Alexander Duyck
>>  wrote:
>>> On Thu, Apr 21, 2016 at 10:21 AM, Babu Moger  wrote:
 Current code writes the tx/rx relaxed order without reading it first.
 This can lead to unintended consequences as we are forcibly writing
 other bits.
>>>
>>> The consequences were very much intended as there are situations where
>>> enabling relaxed ordering can lead to data corruption.
>>>
 We noticed this problem while testing VF driver on sparc. Relaxed
 order settings for rx queue were all messed up which was causing
 performance drop with VF interface.
>>>
>>> What additional relaxed ordering bits are you enabling on Sparc?  I'm
>>> assuming it is just the Rx data write back but I want to verify.
>>>
 Fixed it by reading the registers first and setting the specific
 bit of interest. With this change we are able to match the bandwidth
 equivalent to PF interface.

 Signed-off-by: Babu Moger 
>>>
>>> Fixed is a relative term here since you are only chasing performance
>>> from what I can tell.  We need to make certain that this doesn't break
>>> the driver on any other architectures by leading to things like data
>>> corruption.
>>>
>>> - Alex
>>
>> It occurs to me that what might be easier is instead of altering the
>> configuration on all architectures you could instead wrap the write so
>> that on SPARC you include the extra bits you need and on all other
>> architectures you leave the write as-is similar to how the code in the
>> ixgbe_start_hw_gen2 only clears the bits if CONFIG_SPARC is not
>> defined.
>
>
> Here are the default values that I see when testing on Sparc.
>
> Default tx value 0x2a00
>
> All below 3 set
> #define IXGBE_DCA_TXCTRL_DESC_RRO_EN (1 << 9) /* Tx rd Desc Relax Order */
> #define IXGBE_DCA_TXCTRL_DESC_WRO_EN (1 << 11) /* Tx Desc writeback RO bit */
> #define IXGBE_DCA_TXCTRL_DATA_RRO_EN (1 << 13) /* Tx rd data Relax Order */
>
> I am not too worried about tx values. I can keep it as it is. It did not
> seem to cause any problems right now.
>
>
> Default rx value 0xb200
>
> All below 3 set plus one more
>
> #define IXGBE_DCA_RXCTRL_DESC_RRO_EN (1 << 9) /* DCA Rx rd Desc Relax Order */
> #define IXGBE_DCA_RXCTRL_DATA_WRO_EN (1 << 13) /* Rx wr data Relax Order */
> #define IXGBE_DCA_RXCTRL_HEAD_WRO_EN (1 << 15) /* Rx wr header RO */

So that looks like the register defaults.  Which based on the released
data-sheet for 82599 are a bit off.  The "one more" bit that is set is
supposed to be written as 0 as per the 82599 datasheet, but it
defaults to 1 for some reason.  On the x540 data-sheet that appears to
be a no-snoop bit that you are enabling which should not be enabled.
It won't necessarily hurt things either though as I believe the
no-snoop bit is not being set in the descriptors.

> Is there a reason to disable IXGBE_DCA_RXCTRL_DATA_WRO_EN and
> IXGBE_DCA_RXCTRL_HEAD_WRO_EN for RX?

In the case of HEAD_WRO_EN it doesn't give us anything because we
don't have packet split/replication enabled anyway.  That feature is
broken on the 82599, and was deprecated some time ago in the ixgbe
driver.

I don't have the fully history on the data writeback but I believe
there was an issue of where some x86 chipsets had issues when the
device enabled relaxed ordering.  That was why relaxed ordering was
disabled on all writes for the device.  I was the one that went
through and re-enabled relaxed ordering on reads from the device so we
at least allowed that much on most architectures.

You would probably only need to add IXGBE_DCA_RXCTRL_DATA_WRO_EN to
the write in the case of CONFIG_SPARC being defined.  Another approach
might be to have a define value that you end up passing that is
defined one way if SPARC and another if a different architecture.

> I would think CONFIG_SPARC should be our last option. What do you think?

I was thinking CONFIG_SPARC would allow us to have feature parity with
the PF.  If you look that is how this issue is solved there in
function ixgbe_start_hw_gen2().  That was why I was thinking that may
be the approach we want to take.  Otherwise we have to write up some
complicated setup where we would have to use the API in order to
determine if the PF has already taken care of this for us which I
would prefer not to have to do.

- Alex

Re: [PATCH] ixgbevf: Fix relaxed order settings in VF driver

[Alexander Duyck]

On Thu, Apr 21, 2016 at 12:39 PM, Babu Moger  wrote:
> Hi Alex,
>
> On 4/21/2016 2:22 PM, Alexander Duyck wrote:
>> On Thu, Apr 21, 2016 at 11:13 AM, Alexander Duyck
>>  wrote:
>>> On Thu, Apr 21, 2016 at 10:21 AM, Babu Moger  wrote:
 Current code writes the tx/rx relaxed order without reading it first.
 This can lead to unintended consequences as we are forcibly writing
 other bits.
>>>
>>> The consequences were very much intended as there are situations where
>>> enabling relaxed ordering can lead to data corruption.
>>>
 We noticed this problem while testing VF driver on sparc. Relaxed
 order settings for rx queue were all messed up which was causing
 performance drop with VF interface.
>>>
>>> What additional relaxed ordering bits are you enabling on Sparc?  I'm
>>> assuming it is just the Rx data write back but I want to verify.
>>>
 Fixed it by reading the registers first and setting the specific
 bit of interest. With this change we are able to match the bandwidth
 equivalent to PF interface.

 Signed-off-by: Babu Moger 
>>>
>>> Fixed is a relative term here since you are only chasing performance
>>> from what I can tell.  We need to make certain that this doesn't break
>>> the driver on any other architectures by leading to things like data
>>> corruption.
>>>
>>> - Alex
>>
>> It occurs to me that what might be easier is instead of altering the
>> configuration on all architectures you could instead wrap the write so
>> that on SPARC you include the extra bits you need and on all other
>> architectures you leave the write as-is similar to how the code in the
>> ixgbe_start_hw_gen2 only clears the bits if CONFIG_SPARC is not
>> defined.
>
>
> Here are the default values that I see when testing on Sparc.
>
> Default tx value 0x2a00
>
> All below 3 set
> #define IXGBE_DCA_TXCTRL_DESC_RRO_EN (1 << 9) /* Tx rd Desc Relax Order */
> #define IXGBE_DCA_TXCTRL_DESC_WRO_EN (1 << 11) /* Tx Desc writeback RO bit */
> #define IXGBE_DCA_TXCTRL_DATA_RRO_EN (1 << 13) /* Tx rd data Relax Order */
>
> I am not too worried about tx values. I can keep it as it is. It did not
> seem to cause any problems right now.
>
>
> Default rx value 0xb200
>
> All below 3 set plus one more
>
> #define IXGBE_DCA_RXCTRL_DESC_RRO_EN (1 << 9) /* DCA Rx rd Desc Relax Order */
> #define IXGBE_DCA_RXCTRL_DATA_WRO_EN (1 << 13) /* Rx wr data Relax Order */
> #define IXGBE_DCA_RXCTRL_HEAD_WRO_EN (1 << 15) /* Rx wr header RO */

So that looks like the register defaults.  Which based on the released
data-sheet for 82599 are a bit off.  The "one more" bit that is set is
supposed to be written as 0 as per the 82599 datasheet, but it
defaults to 1 for some reason.  On the x540 data-sheet that appears to
be a no-snoop bit that you are enabling which should not be enabled.
It won't necessarily hurt things either though as I believe the
no-snoop bit is not being set in the descriptors.

> Is there a reason to disable IXGBE_DCA_RXCTRL_DATA_WRO_EN and
> IXGBE_DCA_RXCTRL_HEAD_WRO_EN for RX?

In the case of HEAD_WRO_EN it doesn't give us anything because we
don't have packet split/replication enabled anyway.  That feature is
broken on the 82599, and was deprecated some time ago in the ixgbe
driver.

I don't have the fully history on the data writeback but I believe
there was an issue of where some x86 chipsets had issues when the
device enabled relaxed ordering.  That was why relaxed ordering was
disabled on all writes for the device.  I was the one that went
through and re-enabled relaxed ordering on reads from the device so we
at least allowed that much on most architectures.

You would probably only need to add IXGBE_DCA_RXCTRL_DATA_WRO_EN to
the write in the case of CONFIG_SPARC being defined.  Another approach
might be to have a define value that you end up passing that is
defined one way if SPARC and another if a different architecture.

> I would think CONFIG_SPARC should be our last option. What do you think?

I was thinking CONFIG_SPARC would allow us to have feature parity with
the PF.  If you look that is how this issue is solved there in
function ixgbe_start_hw_gen2().  That was why I was thinking that may
be the approach we want to take.  Otherwise we have to write up some
complicated setup where we would have to use the API in order to
determine if the PF has already taken care of this for us which I
would prefer not to have to do.

- Alex