Re: [RFC LINUX PATCH] dt: xilinx: xadc: provision to control clock frequency

[Sören Brinkmann]

On Wed, 2015-12-23 at 03:58PM +0530, Ranjit Waghmode wrote:
> This patch adds parameter to the xilinx-xadc node for controlling
> clock frequency.
> 
> Following are the possible options for user to control the frequency:
>   * 00 : 1/2 of clock frequency
>   * 01 : 1/4 of clock frequency
>   * 10 : 1/8 of clock frequency
>   * 11 : 1/16 of clock frequency

How is this chosen? Are these just arbitrary values that you need for
some use-case? Or are these the options HW allows?

> 
> So this patch adds parameter tck-rate to set user defined values from
> above pool to control the clock frequency.

This is no longer describing HW, but configuring driver/HW. Why does
this have to be in DT? Why can't the driver request/set the frequency as
operation requires it?

Thanks,
Sören
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM64: ZynqMP: DT: Fix GIC's 'reg' property

[Sören Brinkmann]

On Tue, 2015-12-15 at 04:01PM +0100, Michal Simek wrote:
> Hi,
> 
> On 15.12.2015 10:14, Sören Brinkmann wrote:
> > On Mon, 2015-12-14 at 05:01PM +, Marc Zyngier wrote:
> >> Mark,
> >>
> >> On 14/12/15 16:46, Mark Rutland wrote:
> >>> On Mon, Dec 14, 2015 at 08:31:40AM -0800, Soren Brinkmann wrote:
> >>>> Signed-off-by: Soren Brinkmann <soren.brinkm...@xilinx.com>
> >>>> ---
> >>>>  arch/arm64/boot/dts/xilinx/zynqmp.dtsi | 6 +++---
> >>>>  1 file changed, 3 insertions(+), 3 deletions(-)
> >>>>
> >>>> diff --git a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi 
> >>>> b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
> >>>> index 857eda5c7217..b5d1facadf16 100644
> >>>> --- a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
> >>>> +++ b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
> >>>> @@ -80,10 +80,10 @@
> >>>>  gic: interrupt-controller@f901 {
> >>>>  compatible = "arm,gic-400", 
> >>>> "arm,cortex-a15-gic";
> >>>>  #interrupt-cells = <3>;
> >>>> -reg = <0x0 0xf901 0x1>,
> >>>> -  <0x0 0xf902f000 0x2000>,
> >>>> +reg = <0x0 0xf901 0x1000>,
> >>>> +  <0x0 0xf902 0x2>,
> >>>><0x0 0xf904 0x2>,
> >>>> -  <0x0 0xf906f000 0x2000>;
> >>>> +  <0x0 0xf906 0x2>;
> >>>
> >>> I'm confused. These sizes don't look right for GIC-400. Is this a custom
> >>> GIC?
> >>
> >> Probably an implementation that obey the SBSA requirement of aliasing
> >> the first 4kB of the CPU interface on a 64kB page, and the second one on
> >> the following 64kB page. See the APM system for an example of such a
> >> thing. I'm more concerned about the GICH region (3rd one), which has no
> >> reason to be bigger than 4kB.
> > 
> > Xilinx didn't publish the memory map yet (at least I didn't see it in the
> > public docs), so, let me give some excerpts:
> > 
> > GICD:
> > GICD_CTLR   0xF901  32  rw  0x  Distributor 
> > Control Register
> > ...
> > GICD_CIDR3  0xF9010FFC  32  ro  0x00B1  Component ID3 
> > Register
> > 
> > GICC:
> > GICC_CTLR   0xF902  32  rw  0x  CPU Interface 
> > Control Register
> > ...
> > GICC_DIR0xF903  32  wo  x   Deactivate Interrupt 
> > Register
> > 
> > GICH:
> > GICH_HCR0xF904  32  rw  0x  Hypervisor 
> > Control Register
> > ...
> > GICH_LR3_Alias7 0xF9050F0C  32  rw  0x  List 
> > Register 3
> > 
> > GICV:
> > GICV_CTLR   0xF906  32  rw  0x  Virtual Machine 
> > Control Register
> > ...
> > GICV_DIR0xF907  32  wo  x   VM Deactivate Interrupt 
> > Register
> > 
> > 
> > Regarding the GICH area, it looks like it starts at 0xF904 and the
> > alias blocks to access the other processor interfaces start at
> > 0xF905.
> > 
> >>
> >>> Did this ever work wit hteh old offsets and sizes?
> >>
> >> It probably dies when trying to use EOImode==1.
> > 
> > Without knowing what parts we really exercise, yes, the system comes up
> > fine so far, but I recently found Linux boot hanging on QEMU and it
> > seemed to be related to time not progressing (fast enough).
> > I found a different DT using the values proposed here and that fixed the
> > hang for me.
> 
> We have discussed this here before with Rob
> https://lkml.org/lkml/2015/2/24/371
> 
> Not sure if there is any fix. It is probably just broken QEMU not DTS
> description in mainline.

Do I understand this correctly?: The GIC IP has more configurable
addresses than the DT binding/driver allow. Thus far people have worked
around this by specifying some funky address in DT relying on the IP
aliasing some addresses?

Thanks,
Sören
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM64: ZynqMP: DT: Fix GIC's 'reg' property

[Sören Brinkmann]

On Mon, 2015-12-14 at 05:01PM +, Marc Zyngier wrote:
> Mark,
> 
> On 14/12/15 16:46, Mark Rutland wrote:
> > On Mon, Dec 14, 2015 at 08:31:40AM -0800, Soren Brinkmann wrote:
> >> Signed-off-by: Soren Brinkmann 
> >> ---
> >>  arch/arm64/boot/dts/xilinx/zynqmp.dtsi | 6 +++---
> >>  1 file changed, 3 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi 
> >> b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
> >> index 857eda5c7217..b5d1facadf16 100644
> >> --- a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
> >> +++ b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
> >> @@ -80,10 +80,10 @@
> >>gic: interrupt-controller@f901 {
> >>compatible = "arm,gic-400", "arm,cortex-a15-gic";
> >>#interrupt-cells = <3>;
> >> -  reg = <0x0 0xf901 0x1>,
> >> -<0x0 0xf902f000 0x2000>,
> >> +  reg = <0x0 0xf901 0x1000>,
> >> +<0x0 0xf902 0x2>,
> >>  <0x0 0xf904 0x2>,
> >> -<0x0 0xf906f000 0x2000>;
> >> +<0x0 0xf906 0x2>;
> > 
> > I'm confused. These sizes don't look right for GIC-400. Is this a custom
> > GIC?
> 
> Probably an implementation that obey the SBSA requirement of aliasing
> the first 4kB of the CPU interface on a 64kB page, and the second one on
> the following 64kB page. See the APM system for an example of such a
> thing. I'm more concerned about the GICH region (3rd one), which has no
> reason to be bigger than 4kB.

Xilinx didn't publish the memory map yet (at least I didn't see it in the
public docs), so, let me give some excerpts:

GICD:
GICD_CTLR   0xF901  32  rw  0x  Distributor 
Control Register
...
GICD_CIDR3  0xF9010FFC  32  ro  0x00B1  Component ID3 
Register

GICC:
GICC_CTLR   0xF902  32  rw  0x  CPU Interface 
Control Register
...
GICC_DIR0xF903  32  wo  x   Deactivate Interrupt 
Register

GICH:
GICH_HCR0xF904  32  rw  0x  Hypervisor 
Control Register
...
GICH_LR3_Alias7 0xF9050F0C  32  rw  0x  List 
Register 3

GICV:
GICV_CTLR   0xF906  32  rw  0x  Virtual Machine 
Control Register
...
GICV_DIR0xF907  32  wo  x   VM Deactivate Interrupt 
Register


Regarding the GICH area, it looks like it starts at 0xF904 and the
alias blocks to access the other processor interfaces start at
0xF905.

> 
> > Did this ever work wit hteh old offsets and sizes?
> 
> It probably dies when trying to use EOImode==1.

Without knowing what parts we really exercise, yes, the system comes up
fine so far, but I recently found Linux boot hanging on QEMU and it
seemed to be related to time not progressing (fast enough).
I found a different DT using the values proposed here and that fixed the
hang for me.

Thanks,
Sören
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM: dt: zynq: Add labels to cpu nodes to allow overriding OPPs.

[Sören Brinkmann]

On Mon, 2015-11-09 at 10:51AM -0800, Moritz Fischer wrote:
> By adding labels to the cpu nodes in the dtsi, a dts that
> includes it can change the OPPs by referencing the cpu0
> through the label.
> 
> Signed-off-by: Moritz Fischer <moritz.fisc...@ettus.com>
Reviewed-by: Sören Brinkmann <soren.brinkm...@xilinx.com>

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

Re: [PATCHv2 1/3] ARM: dt: fpga: Added binding docs for Xilinx Zynq FPGA manager.

[Sören Brinkmann]

On Sun, 2015-10-18 at 12:51PM -0500, Josh Cartwright wrote:
> On Fri, Oct 16, 2015 at 03:42:28PM -0700, Moritz Fischer wrote:
> > Signed-off-by: Moritz Fischer <moritz.fisc...@ettus.com>
> > ---
> > 
> > v2:
> >  - Clock names are now a required property
> >  - Removed interrupt-parent property
> > 
> > ---
> >  .../devicetree/bindings/fpga/xilinx-zynq-fpga-mgr.txt | 19 
> > +++
> >  1 file changed, 19 insertions(+)
> >  create mode 100644 
> > Documentation/devicetree/bindings/fpga/xilinx-zynq-fpga-mgr.txt
> > 
> > diff --git 
> > a/Documentation/devicetree/bindings/fpga/xilinx-zynq-fpga-mgr.txt 
> > b/Documentation/devicetree/bindings/fpga/xilinx-zynq-fpga-mgr.txt
> > new file mode 100644
> > index 000..7018aa8
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/fpga/xilinx-zynq-fpga-mgr.txt
> > @@ -0,0 +1,19 @@
> > +Xilinx Zynq FPGA Manager
> > +
> > +Required properties:
> > +- compatible:  should contain "xlnx,zynq-devcfg-1.0"
> > +- reg: base address and size for memory mapped io
> > +- interrupts:  interrupt for the FPGA manager device

If we are that picky, this is technically an interrupt specifier :)

> > +- clocks:  phandle for clocks required operation
> 
> Technically a "clock specifier", but other than that:
> 
> Reviewed-by: Josh Cartwright <jo...@ni.com>
Reviewed-by: Sören Brinkmann <soren.brinkm...@xilinx.com>

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

Re: [PATCH 2/5] gpio/xilinx: enable for MIPS

[Sören Brinkmann]

On Wed, 2015-10-14 at 01:51PM +0100, Zubair Lutfullah Kakakhel wrote:
> MIPSfpga uses the axi gpio controller. Enable the driver for MIPS.
> 
> Signed-off-by: Zubair Lutfullah Kakakhel 
> ---
>  drivers/gpio/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index 8949b3f..58e9afd 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -508,7 +508,7 @@ config GPIO_XGENE_SB
>  
>  config GPIO_XILINX
>   tristate "Xilinx GPIO support"
> - depends on OF_GPIO && (PPC || MICROBLAZE || ARCH_ZYNQ || X86)
> + depends on OF_GPIO && (PPC || MICROBLAZE || ARCH_ZYNQ || X86 || MIPS)

Hmm, in general, this driver is hopefully generic enough that it doesn't
have any real architecture dependencies. And I suspect, we want to
enable this driver for ARM64 for ZynqMP soon too. Should we probably
drop these arch dependencies completely? It seems to become quite a long list.

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

Re: [PATCH 2/5] gpio/xilinx: enable for MIPS

[Sören Brinkmann]

On Wed, 2015-10-14 at 05:57PM +0200, Lars-Peter Clausen wrote:
> On 10/14/2015 05:18 PM, Sören Brinkmann wrote:
> > On Wed, 2015-10-14 at 01:51PM +0100, Zubair Lutfullah Kakakhel wrote:
> >> MIPSfpga uses the axi gpio controller. Enable the driver for MIPS.
> >>
> >> Signed-off-by: Zubair Lutfullah Kakakhel <zubair.kakak...@imgtec.com>
> >> ---
> >>  drivers/gpio/Kconfig | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> >> index 8949b3f..58e9afd 100644
> >> --- a/drivers/gpio/Kconfig
> >> +++ b/drivers/gpio/Kconfig
> >> @@ -508,7 +508,7 @@ config GPIO_XGENE_SB
> >>  
> >>  config GPIO_XILINX
> >>tristate "Xilinx GPIO support"
> >> -  depends on OF_GPIO && (PPC || MICROBLAZE || ARCH_ZYNQ || X86)
> >> +  depends on OF_GPIO && (PPC || MICROBLAZE || ARCH_ZYNQ || X86 || MIPS)
> > 
> > Hmm, in general, this driver is hopefully generic enough that it doesn't
> > have any real architecture dependencies. And I suspect, we want to
> > enable this driver for ARM64 for ZynqMP soon too. Should we probably
> > drop these arch dependencies completely? It seems to become quite a long 
> > list.
> 
> I've been thinking about this a while ago. This is certainly not the only
> driver affected by this problem. But the thing is people always complain if
> new symbols become visable in Kconfig that don't apply to their platform.
> 
> Maybe we should introduce a HAS_REPROGRAMABLE_LOGIC (or similar) feature
> Kconfig symbol and let platforms which have a FPGA select it and let drivers
> for FPGA peripherals depend on it.

Sounds like a good idea to me. But, does that work for all use-cases.
E.g. if you plug some PCIe card with an FPGA into an x86(_64) machine.
That would allow you to use those drivers, but I'm not sure how that
could pull in the new config symbol.

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

Re: [PATCH 2/5] gpio/xilinx: enable for MIPS

[Sören Brinkmann]

On Wed, 2015-10-14 at 07:24PM +0200, Lars-Peter Clausen wrote:
> On 10/14/2015 06:54 PM, Sören Brinkmann wrote:
> > On Wed, 2015-10-14 at 05:57PM +0200, Lars-Peter Clausen wrote:
> >> On 10/14/2015 05:18 PM, Sören Brinkmann wrote:
> >>> On Wed, 2015-10-14 at 01:51PM +0100, Zubair Lutfullah Kakakhel wrote:
> >>>> MIPSfpga uses the axi gpio controller. Enable the driver for MIPS.
> >>>>
> >>>> Signed-off-by: Zubair Lutfullah Kakakhel <zubair.kakak...@imgtec.com>
> >>>> ---
> >>>>  drivers/gpio/Kconfig | 2 +-
> >>>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> >>>> index 8949b3f..58e9afd 100644
> >>>> --- a/drivers/gpio/Kconfig
> >>>> +++ b/drivers/gpio/Kconfig
> >>>> @@ -508,7 +508,7 @@ config GPIO_XGENE_SB
> >>>>  
> >>>>  config GPIO_XILINX
> >>>>  tristate "Xilinx GPIO support"
> >>>> -depends on OF_GPIO && (PPC || MICROBLAZE || ARCH_ZYNQ || X86)
> >>>> +depends on OF_GPIO && (PPC || MICROBLAZE || ARCH_ZYNQ || X86 || 
> >>>> MIPS)
> >>>
> >>> Hmm, in general, this driver is hopefully generic enough that it doesn't
> >>> have any real architecture dependencies. And I suspect, we want to
> >>> enable this driver for ARM64 for ZynqMP soon too. Should we probably
> >>> drop these arch dependencies completely? It seems to become quite a long 
> >>> list.
> >>
> >> I've been thinking about this a while ago. This is certainly not the only
> >> driver affected by this problem. But the thing is people always complain if
> >> new symbols become visable in Kconfig that don't apply to their platform.
> >>
> >> Maybe we should introduce a HAS_REPROGRAMABLE_LOGIC (or similar) feature
> >> Kconfig symbol and let platforms which have a FPGA select it and let 
> >> drivers
> >> for FPGA peripherals depend on it.
> > 
> > Sounds like a good idea to me. But, does that work for all use-cases.
> > E.g. if you plug some PCIe card with an FPGA into an x86(_64) machine.
> > That would allow you to use those drivers, but I'm not sure how that
> > could pull in the new config symbol.
> 
> Hm, right. We could also make it a user-selectable config symbol. In that
> case you only need to disable one symbol when you don't have FPGA support
> rather than one for each driver. Although I'm not quite sure where to put
> such a symbol.

Eventually, the FPGA manager subsystem could probably provide some high
level config symbols. Though, it is probably also not given that every
FPGA-enabled platform needs the FPGA manager.

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

Re: [PATCH 2/3] dts: Updated devicetree bindings for Zynq 7000 platform

[Sören Brinkmann]

On Fri, 2015-10-09 at 12:45AM +0200, Moritz Fischer wrote:
> Added addtional bindings required for FPGA Manager operation
> of the Xilinx Zynq Devc configuration interface.
> 
> Signed-off-by: Moritz Fischer <moritz.fisc...@ettus.com>
Reviewed-by: Sören Brinkmann <soren.brinkm...@xilinx.com>

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

Re: [PATCH 1/3] doc: dt: fpga: Added Documentation for Xilinx Zynq FPGA manager.

[Sören Brinkmann]

On Fri, 2015-10-09 at 12:45AM +0200, Moritz Fischer wrote:
> Signed-off-by: Moritz Fischer 
> ---
>  .../bindings/fpga/xilinx-zynq-fpga-mgr.txt | 26 
> ++
>  1 file changed, 26 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/fpga/xilinx-zynq-fpga-mgr.txt
> 
> diff --git a/Documentation/devicetree/bindings/fpga/xilinx-zynq-fpga-mgr.txt 
> b/Documentation/devicetree/bindings/fpga/xilinx-zynq-fpga-mgr.txt
> new file mode 100644
> index 000..82ffda8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/fpga/xilinx-zynq-fpga-mgr.txt
> @@ -0,0 +1,26 @@
> +Xilinx Zynq FPGA Manager
> +
> +Required properties:
> +- compatible:should contain "xlnx,zynq-devcfg-1.0"
> +- reg:   base address and size for memory mapped io
> +- interrupt parent:  interrupt source phandle
> +- interrupts:interrupt for the FPGA manager device
> +- clocks:phandle for clocks required operation
> +- syscon:phandle for access to SLCR registers
> +
> +Optional properties:
> +- clock-names:   names for clocks

Is it optional? Currently, there is only one clock input, so a match
without specifying a clock name should work making this optional. But in
your implementation, you do specify a clock name in devm_clk_get(). I'm
not entirely sure, but that call might fail if it doesn't find the
corresponding clock-names property.
I think, either we should make this required and list the required
entries here. Or the implementation probably needs to drop the clock
name when looking up its input clock.

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

Re: [PATCH v2 1/2] mmc: sdhci-of-arasan: add phy support for sdhci-of-arasan

[Sören Brinkmann]

On Tue, 2015-09-15 at 09:07AM +0800, Shawn Lin wrote:
> On 2015/9/14 23:07, Sören Brinkmann wrote:
> >Hi Shawn,
> >
> >overall, it looks good to me. I have some questions though.
> >
> >On Mon, 2015-09-14 at 02:29PM +0800, Shawn Lin wrote:
> 
> [...]
> 
> >>+err_phy_exit:
> >>+   phy_init(phy);
> >
> >Just to confirm, are these actions in the error path correct? E.g.
> >if the power_off() call fails, is it safe to call power_on()? Isn't
> >the phy still powered on? (this would apply to other error paths too)
> >
> 
> Cool question!
> 
> While writing this, I had read generic phy stuffs deliberately to find a
> solution for a case: how to deal with ping-pong fails? In another word, if
> power_off call fails, then we should call power_on, but unfortunately it
> fails again then we call power_off... so endless nested err handling... No
> answer yet.
> 
> So, I assumed two cases happened when power_off call fails:
> (1) *real power_off* is done, but some other stuffs in the calling path
> fail, so phy is really power_off in theory. We need to power_on it again,
> but if it fail, we don't know PHY is on or off since we don't know power_on
> fails for what? *real power on* ? some other stuffs?
> 
> (2) *real power_off* isn't completed, so indeed it's *still* in power_on
> state. The reason we never need to check the return value of power_on cross
> the err handling is that whether power_on call successfully or not, it's
> always make phy in power_on state.
> 
> Now, let's think about case(1).
> After reading dozens of sample codes(such as USB, UFS, MBUS) that adopt
> generic phy framework for PHY management, real thing should be like that:
> they NEVER deal with case(1).
> 
> It's a trick of sub-phy drivers themself. power_on/off calling path return
> err for two case:
> <1>  phy_runtime callback fails. It's after *real power on/off*, so
> definitely *real power on/off* is conpleted. That is the case(2) I mentioned
> above.
> <2>  sub-phy drivers return err for  phy->ops->power_off(phy); Look
> into all the sub-phy drivers twice, we find that they always return success
> for phy->ops->power_off hook. Why? Because all of them
> write registers to enable/disable something, always consider things going
> well. Actually if we write value into a register, we have to think that it's
> functional.
> 
> Anyway, back to this patch.
> Indeed we also write value into arasan phy's register to do
> phy_power_on/off/init/exit to make things work. Right, we return success
> state for all of these them just as all the other sub-phy drivers do.
> 
> Feel free to let me know if I make mistakes or misunderstanding above.
> 
> >>+   return ret;
> >>+}
> 
> [...]
> 
> >>+   }
> >>+   }
> >
> >I assume you looked at options for having the error paths in a
> >consolidated location? I guess this may be the nicest solution since all
> >of this is in this conditional block?
> >
> 
> yep, otherwise we should add some *if* statements to check sdhci_arasan->phy
> cross the err handles. And I intent to strictly limit
> the phy stuffs under the scope of arasan,sdhci-5.1 currently.
> 
> >Feel free to add my
> >Acked-by: Sören Brinkmann <soren.brinkm...@xilinx.com>
> >
> 
> Thanks, Sören. :)

Makes all sense to me. Thanks for all the details.

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

Re: [PATCH 0/7] Phy and mdiobus fixes

[Sören Brinkmann]

Hi Russell,

On Fri, 2015-09-18 at 10:56AM +0100, Russell King - ARM Linux wrote:
> Sorry guys, some of you will get the patches twice, as Sören's name
> in the header caused vger to reject all the patches.

That is the first time I hear about an issue like that. I've been
receiving patches fine thus far and nobody reported any rejections (by
vger) to me. Is it some bounce on Xilinx/my side or is vger suddenly
rejecting non-ascii chars or is something in the mail processing chain
not properly encoding those chars?
Please, let me know if I can help with the problem.

Thanks,
Sören
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 3/3] devicetree: macb: Add optional property tsu-clk

[Sören Brinkmann]

On Mon, 2015-09-14 at 09:58AM +0200, Boris Brezillon wrote:
> Hi Harini,
> 
> On Mon, 14 Sep 2015 09:39:05 +0530
> Harini Katakam <harinikatakamli...@gmail.com> wrote:
> 
> > On Fri, Sep 11, 2015 at 10:22 PM, Sören Brinkmann
> > <soren.brinkm...@xilinx.com> wrote:
> > > Hi Harini,
> > >
> > > On Fri, 2015-09-11 at 01:27PM +0530, Harini Katakam wrote:
> > >> Add TSU clock frequency to be used for 1588 support in macb driver.
> > >>
> > >> Signed-off-by: Harini Katakam <hari...@xilinx.com>
> > >> ---
> > >>  Documentation/devicetree/bindings/net/macb.txt |3 +++
> > >>  1 file changed, 3 insertions(+)
> > >>
> > >> diff --git a/Documentation/devicetree/bindings/net/macb.txt 
> > >> b/Documentation/devicetree/bindings/net/macb.txt
> > >> index b5d7976..f7c0ea8 100644
> > >> --- a/Documentation/devicetree/bindings/net/macb.txt
> > >> +++ b/Documentation/devicetree/bindings/net/macb.txt
> > >> @@ -19,6 +19,9 @@ Required properties:
> > >>   Optional elements: 'tx_clk'
> > >>  - clocks: Phandles to input clocks.
> > >>
> > >> +Optional properties:
> > >> +- tsu-clk: Time stamp unit clock frequency used.
> > >
> > > Why are we not using the CCF and a clk_get_rate() in the driver?
> > >
> > 
> > If the clock source was only internal, we could use this
> > approach as usual. But TSU clock can be configured to
> > come from an external clock source or internal.
> 
> How about declaring a fixed-rate clk [1] if it comes from an external
> clk, and using a clk driver for the internal clk case?
> This way you'll be able to use the clk API (including the
> clk_get_rate() function) instead of introducing a new way to retrieve a
> clk frequency.

Right. Also, Zynq does already support external clock inputs (actually,
every clock originates from some external clock/oscillator at some
point). Maybe that code needs some additions to handle the TSU clock
too. But either way, I can't see why a clock cannot be modeled using the
CCF.

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

Re: [PATCH v2 2/2] Documentation: bindings: add description of phy for sdhci-of-arasan

[Sören Brinkmann]

On Mon, 2015-09-14 at 02:29PM +0800, Shawn Lin wrote:
> This patch adds phys and phy-names for sdhci-of-arasan as required
> properties for arasan,sdhci-5.1, and details the example as well.
> 
> Signed-off-by: Shawn Lin <shawn@rock-chips.com>
Acked-by: Sören Brinkmann <soren.brinkm...@xilinx.com>

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

Re: [PATCH v2 1/2] mmc: sdhci-of-arasan: add phy support for sdhci-of-arasan

[Sören Brinkmann]

dev_err(>dev, "phy_power_on err.\n");
> + phy_power_off(sdhci_arasan->phy);
> +     goto clk_dis_ahb;
> + }
> +
> + ret = phy_init(sdhci_arasan->phy);
> + if (ret < 0) {
> + dev_err(>dev, "phy_init err.\n");
> + phy_exit(sdhci_arasan->phy);
> + phy_power_off(sdhci_arasan->phy);
> + goto clk_dis_ahb;
> + }
> + }

I assume you looked at options for having the error paths in a
consolidated location? I guess this may be the nicest solution since all
of this is in this conditional block?

Feel free to add my
Acked-by: Sören Brinkmann <soren.brinkm...@xilinx.com>

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

Re: [PATCH 2/2] Documentation: bindings: add description of phy for sdhci-of-arasan

[Sören Brinkmann]

Hi Shawn,

On Fri, 2015-09-11 at 04:55PM +0800, Shawn Lin wrote:
> This patch adds phys and phy-names for sdhci-of-arasan as optional
> properties, and details the example as well.
> 
> Signed-off-by: Shawn Lin 
> ---
> 
>  Documentation/devicetree/bindings/mmc/arasan,sdhci.txt | 10 --
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt 
> b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> index da541c3..0264d5f 100644
> --- a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> +++ b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> @@ -1,11 +1,12 @@
>  Device Tree Bindings for the Arasan SDHCI Controller
>  
> -  The bindings follow the mmc[1], clock[2] and interrupt[3] bindings. Only
> -  deviations are documented here.
> +  The bindings follow the mmc[1], clock[2], interrupt[3] and phy[4] bindings.
> +  Only deviations are documented here.
>  
>[1] Documentation/devicetree/bindings/mmc/mmc.txt
>[2] Documentation/devicetree/bindings/clock/clock-bindings.txt
>[3] Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
> +  [4] Documentation/devicetree/bindings/phy/phy-bindings.txt
>  
>  Required Properties:
>- compatible: Compatibility string. Must be 'arasan,sdhci-8.9a' or
> @@ -16,6 +17,9 @@ Required Properties:
>- interrupts: Interrupt specifier
>- interrupt-parent: Phandle for the interrupt controller that services
> interrupts for this device.
> +Optional Properties:
> +  - phys: From PHY bindings: Phandle for the Generic PHY for arasan.
> +  - phy-names:  MUST be "phy_arasan".

This might be a dumb question, but, is the external phy actually
optional? Or is it mandatory for certain implementations of the IP.
In the latter case, it should probably be a mandatory property depending
on the compatible string matching an implementation that requires an
external phy (accordingly, the implementation might have to treat it
that way too).

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

Re: [RFC PATCH 3/3] devicetree: macb: Add optional property tsu-clk

[Sören Brinkmann]

Hi Harini,

On Fri, 2015-09-11 at 01:27PM +0530, Harini Katakam wrote:
> Add TSU clock frequency to be used for 1588 support in macb driver.
> 
> Signed-off-by: Harini Katakam 
> ---
>  Documentation/devicetree/bindings/net/macb.txt |3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/macb.txt 
> b/Documentation/devicetree/bindings/net/macb.txt
> index b5d7976..f7c0ea8 100644
> --- a/Documentation/devicetree/bindings/net/macb.txt
> +++ b/Documentation/devicetree/bindings/net/macb.txt
> @@ -19,6 +19,9 @@ Required properties:
>   Optional elements: 'tx_clk'
>  - clocks: Phandles to input clocks.
>  
> +Optional properties:
> +- tsu-clk: Time stamp unit clock frequency used.

Why are we not using the CCF and a clk_get_rate() in the driver?

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

Re: [RFCv3 4/4] ARM: zynq: Select ARCH_HAS_RESET_CONTROLLER

[Sören Brinkmann]

Hi Philipp,

On Tue, 2015-08-04 at 10:38AM +0200, Philipp Zabel wrote:
 Hi,
 
 Am Freitag, den 31.07.2015, 09:47 -0700 schrieb Sören Brinkmann:
  On Fri, 2015-07-31 at 10:09AM +0200, Michal Simek wrote:
 [...]
   Reviewed-by: Michal Simek michal.si...@xilinx.com
  Acked-by: Sören Brinkmann soren.brinkm...@xilinx.com
 
 I'll assume these apply to the whole series and queue patches 1 and 3.

For me, yes, that was meant for the whole series. Sorry, for not making
that obvious.

Thanks,
Sören
--
To unsubscribe from this list: send the line unsubscribe devicetree in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFCv3 4/4] ARM: zynq: Select ARCH_HAS_RESET_CONTROLLER

[Sören Brinkmann]

On Fri, 2015-07-31 at 10:09AM +0200, Michal Simek wrote:
 On 07/31/2015 03:13 AM, Moritz Fischer wrote:
  Signed-off-by: Moritz Fischer moritz.fisc...@ettus.com
  ---
   arch/arm/mach-zynq/Kconfig | 1 +
   1 file changed, 1 insertion(+)
  
  diff --git a/arch/arm/mach-zynq/Kconfig b/arch/arm/mach-zynq/Kconfig
  index 78e5e00..77d7df7 100644
  --- a/arch/arm/mach-zynq/Kconfig
  +++ b/arch/arm/mach-zynq/Kconfig
  @@ -1,5 +1,6 @@
   config ARCH_ZYNQ
  bool Xilinx Zynq ARM Cortex A9 Platform if ARCH_MULTI_V7
  +   select ARCH_HAS_RESET_CONTROLLER
  select ARCH_SUPPORTS_BIG_ENDIAN
  select ARM_AMBA
  select ARM_GIC
  
 
 Reviewed-by: Michal Simek michal.si...@xilinx.com
Acked-by: Sören Brinkmann soren.brinkm...@xilinx.com

I personally, would prefer to decouple the logical outputs of the
reset-controller from the HW. But, as Moritz pointed out, that seems
rather uncommon for reset controllers. I think this is good to go.

Thanks,
Sören
--
To unsubscribe from this list: send the line unsubscribe devicetree in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFCv2 1/3] docs: dts: Added documentation for Xilinx Zynq Reset Controller bindings.

[Sören Brinkmann]

On Tue, 2015-07-28 at 11:14PM -0700, Moritz Fischer wrote:
 Hi Sören,
 
 On Tue, Jul 28, 2015 at 3:53 PM, Sören Brinkmann
 soren.brinkm...@xilinx.com wrote:
  On Mon, 2015-07-27 at 09:52PM -0700, Moritz Fischer wrote:
  Hi Sören,
 
  thanks for your feedback.
 
  On Mon, Jul 27, 2015 at 7:58 PM, Sören Brinkmann
  soren.brinkm...@xilinx.com wrote:
   Hi Moritz,
  
   On Fri, 2015-07-24 at 05:21PM -0700, Moritz Fischer wrote:
   Signed-off-by: Moritz Fischer moritz.fisc...@ettus.com
   ---
Documentation/devicetree/bindings/reset/zynq-reset-pl.txt | 13 
   +
1 file changed, 13 insertions(+)
create mode 100644 
   Documentation/devicetree/bindings/reset/zynq-reset-pl.txt
  
   diff --git a/Documentation/devicetree/bindings/reset/zynq-reset-pl.txt 
   b/Documentation/devicetree/bindings/reset/zynq-reset-pl.txt
   new file mode 100644
   index 000..ac4499e
   --- /dev/null
   +++ b/Documentation/devicetree/bindings/reset/zynq-reset-pl.txt
   @@ -0,0 +1,13 @@
   +Xilinx Zynq PL Reset Manager
   +
   +Required properties:
   +- compatible: xlnx,zynq-reset-pl
   +- syscon slcr;
   +- #reset-cells: 1
   +
   +Example:
   + rstc: rstc@240 {
   + #reset-cells = 1;
   + compatible = xlnx,zynq-reset-pl;
   + syscon = slcr;
   + };
  
   I think you also have to add the outputs and make them part of the
   binding. Otherwise you'd have to read the implementation to find
   out what device should be hooked up to which output of the 
   reset-controller.
 
  Is something like this what you had in mind? I had that prepared for
  the next round of patches:
 
  Reset outputs:
   0  : soft reset
   32 : ddr reset
   64 : topsw reset
   96 : dmac reset
   128: usb0 reset
   129: usb1 reset
   160: gem0 reset
   161: gem1 reset
   164: gem0 rx reset
   165: gem1 rx reset
   166: gem0 ref reset
   167: gem1 ref reset
   192: sdio0 reset
   193: sdio1 reset
   196: sdio0 ref reset
   197: sdio1 ref reset
   224: spi0 reset
   225: spi1 reset
   226: spi0 ref reset
   227: spi1 ref reset
   256: can0 reset
   257: can1 reset
   258: can0 ref reset
   259: can1 ref reset
   288: i2c0 reset
   289: i2c1 reset
   320: uart0 reset
   321: uart1 reset
   322: uart0 ref reset
   323: uart1 ref reset
   352: gpio reset
   384: lqspi reset
   385: qspi ref reset
   416: smc reset
   417: smc ref reset
   448: ocm reset
   512: fpga0 out reset
   513: fpga1 out reset
   514: fpga2 out reset
   515: fpga3 out reset
   544: a9 reset 0
   545: a9 reset 1
   552: peri reset
 
  Basically, yes. I guess the gaps are due to directly mapping this number
  to bank and bit instead of doing some more complex mapping in between?
  I'm not sure whether I like this :) I guess if a number is off the
  driver would still toggle the addressed bit?
 
 My assumption was that people would use a #include
 dt-bindings/xlnx,zynq-reset.h in their dts. Assuming I got the
 numbers in there right this makes it hard to misuse by accident.
 I'm not saying it's perfect ...

Michal always turned down the #include patches with the argument of
re-using the dts files outside of the Linux sources where those includes
etc may not be available in this form.

 
  I'm not sure, is it worth
  to do some explicit mapping from logical outputs to a physical reset? It
  seems it would be a little safer since it would be easy to check that
  the addressed reset is valid and there wouldn't be any reserved/invalid
  bits be toggled. Also, it would make the outputs in here a continuous
  series of numbers without these gaps. Not sure though whether
  it's worth the additional complexity in the implementation.
 
 So your suggestion is to have a large switch case kind of thingy? I
 thought about it and it seemed complex as there's quite a bunch of
 resets with gaps. Do you know of a driver that does something similar?

Yeah, that was what I had in mind. Some big switch-case lookup that maps
a logical reset number from DT to the HW.
No, I haven't looked around what other drivers do. So, probably the
right thing is to just do what everybody else does.
I was more thinking about how easy it might be to re-use the driver for
the next-gen device.

 When I wrote this I looked at the other reset drivers and figured they
 all had this kind of bank mapping of sorts so I assumed that's how
 people usually do it.

Yeah, I don't think we should make things overly complicated without a
good reason. So, unless DT or reset folks have any objections, I'm fine
with it.

Thanks,
Sören
--
To unsubscribe from this list: send the line unsubscribe devicetree in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFCv2 1/3] docs: dts: Added documentation for Xilinx Zynq Reset Controller bindings.

[Sören Brinkmann]

On Mon, 2015-07-27 at 09:52PM -0700, Moritz Fischer wrote:
 Hi Sören,
 
 thanks for your feedback.
 
 On Mon, Jul 27, 2015 at 7:58 PM, Sören Brinkmann
 soren.brinkm...@xilinx.com wrote:
  Hi Moritz,
 
  On Fri, 2015-07-24 at 05:21PM -0700, Moritz Fischer wrote:
  Signed-off-by: Moritz Fischer moritz.fisc...@ettus.com
  ---
   Documentation/devicetree/bindings/reset/zynq-reset-pl.txt | 13 
  +
   1 file changed, 13 insertions(+)
   create mode 100644 
  Documentation/devicetree/bindings/reset/zynq-reset-pl.txt
 
  diff --git a/Documentation/devicetree/bindings/reset/zynq-reset-pl.txt 
  b/Documentation/devicetree/bindings/reset/zynq-reset-pl.txt
  new file mode 100644
  index 000..ac4499e
  --- /dev/null
  +++ b/Documentation/devicetree/bindings/reset/zynq-reset-pl.txt
  @@ -0,0 +1,13 @@
  +Xilinx Zynq PL Reset Manager
  +
  +Required properties:
  +- compatible: xlnx,zynq-reset-pl
  +- syscon slcr;
  +- #reset-cells: 1
  +
  +Example:
  + rstc: rstc@240 {
  + #reset-cells = 1;
  + compatible = xlnx,zynq-reset-pl;
  + syscon = slcr;
  + };
 
  I think you also have to add the outputs and make them part of the
  binding. Otherwise you'd have to read the implementation to find
  out what device should be hooked up to which output of the reset-controller.
 
 Is something like this what you had in mind? I had that prepared for
 the next round of patches:
 
 Reset outputs:
  0  : soft reset
  32 : ddr reset
  64 : topsw reset
  96 : dmac reset
  128: usb0 reset
  129: usb1 reset
  160: gem0 reset
  161: gem1 reset
  164: gem0 rx reset
  165: gem1 rx reset
  166: gem0 ref reset
  167: gem1 ref reset
  192: sdio0 reset
  193: sdio1 reset
  196: sdio0 ref reset
  197: sdio1 ref reset
  224: spi0 reset
  225: spi1 reset
  226: spi0 ref reset
  227: spi1 ref reset
  256: can0 reset
  257: can1 reset
  258: can0 ref reset
  259: can1 ref reset
  288: i2c0 reset
  289: i2c1 reset
  320: uart0 reset
  321: uart1 reset
  322: uart0 ref reset
  323: uart1 ref reset
  352: gpio reset
  384: lqspi reset
  385: qspi ref reset
  416: smc reset
  417: smc ref reset
  448: ocm reset
  512: fpga0 out reset
  513: fpga1 out reset
  514: fpga2 out reset
  515: fpga3 out reset
  544: a9 reset 0
  545: a9 reset 1
  552: peri reset

Basically, yes. I guess the gaps are due to directly mapping this number
to bank and bit instead of doing some more complex mapping in between?
I'm not sure whether I like this :) I guess if a number is off the
driver would still toggle the addressed bit? I'm not sure, is it worth
to do some explicit mapping from logical outputs to a physical reset? It
seems it would be a little safer since it would be easy to check that
the addressed reset is valid and there wouldn't be any reserved/invalid
bits be toggled. Also, it would make the outputs in here a continuous
series of numbers without these gaps. Not sure though whether
it's worth the additional complexity in the implementation.

Thanks,
Sören
--
To unsubscribe from this list: send the line unsubscribe devicetree in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFCv2 3/3] reset: reset-zynq: Adding support for Xilinx Zynq reset controller.

[Sören Brinkmann]

On Tue, 2015-07-28 at 07:05AM -0700, Moritz Fischer wrote:
 Philip,
 
 thanks for your review :)
 
 On Tue, Jul 28, 2015 at 1:38 AM, Philipp Zabel p.za...@pengutronix.de wrote:
  Hi Moritz,
 
  Am Freitag, den 24.07.2015, 17:21 -0700 schrieb Moritz Fischer:
  This adds a reset controller driver to control the Xilinx Zynq
  SoC's various resets.
 
  Signed-off-by: Moritz Fischer moritz.fisc...@ettus.com
[...]
  +
  +/* Offsets into SLCR regmap */
  +#define SLCR_RST_CTRL_OFFSET 0x200 /* FPGA Software Reset Control */
 
  Maybe get this value from the reg property? I'm not sure if this is ever
  expected to change.
 I don't think it's going to change. Is there a reason to only expose
 part of the resets?

I think all other users of the syscon (in the Zynq DT) use the 'reg' property
to retrieve an offset into the SLCR. We should probably do that here too and
remove the #define. Who knows, maybe this driver is reusable with some
modifications for the Zynq MPSoC.

 
  +#define NBANKS   18
 
  reg = 0x200 0x50 says there are two more registers, are those not used?
 
 Should be 0x48, you're right. Michal had suggested 0x50, but the last
 two regs are not really resets.
 
[...]
  +static int zynq_reset_status(struct reset_controller_dev *rcdev,
  +  unsigned long id)
  +{
  + struct zynq_reset_data *priv = to_zynq_reset_data(rcdev);
  +
  + int bank = id / BITS_PER_LONG;
  + int offset = id % BITS_PER_LONG;
  + u32 reg;
  +
  + regmap_read(priv-slcr, SLCR_RST_CTRL_OFFSET + (bank * 4), reg);
  +
  + return !(reg  BIT(offset));
  +}
 
 Will change to:
 ret = regmap_read(priv-slcr, SLCR_RST_CTRL_OFFSET + (bank * 4), 
 reg);
 if (ret)
 return ret;
 else
 return !!(reg  BIT(offset));
 
 the single '!' was a typo ...

You have an early return on error in the if-branch. No need for the
else.

 
 
  Do I understand this correctly, you write 1 to assert the reset, but the
  register reads 0 while the reset is asserted and 1 otherwise?
  Also note that reset_status may return negative ERRNO, so for offset ==
  31 you should not return (131).
 
  +static const struct reset_control_ops zynq_reset_ops = {
  + .assert = zynq_reset_assert,
  + .deassert   = zynq_reset_deassert,
  + .status = zynq_reset_status,
  +};
  +
  +static int zynq_reset_probe(struct platform_device *pdev)
  +{
  + struct zynq_reset_data *priv;
  +
  + priv = devm_kzalloc(pdev-dev, sizeof(*priv), GFP_KERNEL);
  + if (!priv)
  + return -ENOMEM;
  + platform_set_drvdata(pdev, priv);
  +
  + priv-slcr = syscon_regmap_lookup_by_phandle(pdev-dev.of_node,
  + syscon);
 
  I'd just use syscon_node_to_regmap(pdev-dev.of_node-parent) here,
  which removes the need for the syscon phandle binding.
 
 See binding doc discussion. I don't have a strong preference either way,
 just tried to be consistent with the pinctrl node.
 We just need a decision one way or the other :)

I personally like the syscon handle better since it would make placing
the node more flexible, while this proposal forces some topology on the
DT. In both cases though, the syscon and this user are tightly coupled
and this driver depends on the syscon. I don't really mind either way -
I think.

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

Re: [RFCv2 1/3] docs: dts: Added documentation for Xilinx Zynq Reset Controller bindings.

[Sören Brinkmann]

Hi Moritz,

On Fri, 2015-07-24 at 05:21PM -0700, Moritz Fischer wrote:
 Signed-off-by: Moritz Fischer moritz.fisc...@ettus.com
 ---
  Documentation/devicetree/bindings/reset/zynq-reset-pl.txt | 13 +
  1 file changed, 13 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/reset/zynq-reset-pl.txt
 
 diff --git a/Documentation/devicetree/bindings/reset/zynq-reset-pl.txt 
 b/Documentation/devicetree/bindings/reset/zynq-reset-pl.txt
 new file mode 100644
 index 000..ac4499e
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/reset/zynq-reset-pl.txt
 @@ -0,0 +1,13 @@
 +Xilinx Zynq PL Reset Manager
 +
 +Required properties:
 +- compatible: xlnx,zynq-reset-pl
 +- syscon slcr;
 +- #reset-cells: 1
 +
 +Example:
 + rstc: rstc@240 {
 + #reset-cells = 1;
 + compatible = xlnx,zynq-reset-pl;
 + syscon = slcr;
 + };

I think you also have to add the outputs and make them part of the
binding. Otherwise you'd have to read the implementation to find
out what device should be hooked up to which output of the reset-controller.

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

Re: [PATCH] devicetree: xilinx: add rtc node to zynqmp dtsi

[Sören Brinkmann]

Hi Suneel,

On Thu, 2015-07-23 at 04:22PM +0530, Suneel Garapati wrote:
 adds rtc controller node to zynqmp devicetree.
 
 Signed-off-by: Suneel Garapati suneel.garap...@xilinx.com
 ---
  arch/arm64/boot/dts/xilinx/zynqmp-ep108.dts | 4 
  arch/arm64/boot/dts/xilinx/zynqmp.dtsi  | 9 +
  2 files changed, 13 insertions(+)
 
 diff --git a/arch/arm64/boot/dts/xilinx/zynqmp-ep108.dts 
 b/arch/arm64/boot/dts/xilinx/zynqmp-ep108.dts
 index 0a3f40e..b48ce47 100644
 --- a/arch/arm64/boot/dts/xilinx/zynqmp-ep108.dts
 +++ b/arch/arm64/boot/dts/xilinx/zynqmp-ep108.dts
 @@ -45,3 +45,7 @@
  uart0 {
   status = okay;
  };
 +
 +rtc {
 + status = okay;
 +};
 \ No newline at end of file
 diff --git a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi 
 b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
 index 11e0b00..b76ede1 100644
 --- a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
 +++ b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
 @@ -220,6 +220,15 @@
   #size-cells = 0;
   };
 
 + rtc: rtc@ffa6 {
 + compatible = xlnx,zynqmp-rtc;
 + status = disabled;

do we need the status disabled here?

 + reg = 0x0 0xffa6 0x100;
 + interrupt-parent = gic;
 + interrupts = 0 26 4, 0 27 4;
 + interrupt-names = alm, sec;

Descriptive names would be nice. What is 'alm' supposed to mean?

Thanks,
Sören
--
To unsubscribe from this list: send the line unsubscribe devicetree in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv7 1/2] dts: Adding docs for Xilinx LogiCORE IP mailbox driver.

[Sören Brinkmann]

On Tue, 2015-07-14 at 06:00PM -0700, Moritz Fischer wrote:
 Signed-off-by: Moritz Fischer moritz.fisc...@ettus.com
 Acked-by: Michal Simek michal.si...@xilinx.com
Acked-by: Sören Brinkmann soren.brinkm...@xilinx.com

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

Re: [PATCHv6 1/2] dts: Adding docs for Xilinx LogiCORE IP mailbox driver.

[Sören Brinkmann]

On Mon, 2015-07-06 at 10:16AM -0700, Moritz Fischer wrote:
 Signed-off-by: Moritz Fischer moritz.fisc...@ettus.com
 Acked-by: Michal Simek michal.si...@xilinx.com
 ---
  .../devicetree/bindings/mailbox/xilinx-mailbox.txt | 44 
 ++
  1 file changed, 44 insertions(+)
  create mode 100644 
 Documentation/devicetree/bindings/mailbox/xilinx-mailbox.txt
 
 diff --git a/Documentation/devicetree/bindings/mailbox/xilinx-mailbox.txt 
 b/Documentation/devicetree/bindings/mailbox/xilinx-mailbox.txt
 new file mode 100644
 index 000..97b81f8
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/mailbox/xilinx-mailbox.txt
 @@ -0,0 +1,44 @@
 +Xilinx Mailbox Driver
 +=
 +
 +Required properties:
 +- compatible   : xlnx,mailbox-2.1.
 +- reg  :  physical base address of the mailbox and length of
 +  memory mapped region.
 +- #mbox-cells  :  common mailbox binding property to identify the number
 +  of cells required for the mailbox specifier, should be  0
 +- clocks   :  phandle to clock provider
 +- clock-names  :  must be 'mbox'
 +
 +Optional properties:
 +- interrupt-parent : interrupt source phandle
 +- interrupts   : interrupt number, The interrupt specifier format
 + depends on the interrupt controller parent.
 +
 +Example:
 + mbox: mailbox@4040 {
 + compatible = xlnx,mailbox-2.1;
 + reg = 0x4040 0x100;
 + interrupt-parent = intc;
 + interrupts = 5;
 + #mbox-cells = 0;
 + clocks = clkc 15;
 + clock-names = mbox;
 + };
 +
 +Mailbox client
 +===
 +mboxes and the optional mbox-names (please see
 +Documentation/devicetree/bindings/mailbox/mailbox.txt for details). Each 
 value
 +of the mboxes property should contain a phandle to the mailbox controller
 +device node and second argument is the channel index. It must be 0 (hardware
 +support only one channel). The equivalent mbox-names property value can be
 +used to give a name to the communication channel to be used by the client 
 user.
 +
 +Example:
 + mclient0: mclient0@400 {
 + compatible = client-1.0;
 + reg = 0x400 0x10;
 + mbox-names = mbox;
 + mboxes = mbox 0;

Sorry, to bring this up again, but I'm confused regarding the #mbox-cells
thing. Above it says #mbox-cells is 0. In that case, shouldn't the
'mboxes' property not only be 'mbox'?

Moreover, the generic mailbox bindings
(https://www.kernel.org/doc/Documentation/devicetree/bindings/mailbox/mailbox.txt)
mandate #mbox-cells to be at least 1.

Does something need to be aligned here?

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

Re: [PATCHv5 1/2] dts: Adding docs for Xilinx LogiCORE IP mailbox driver.

[Sören Brinkmann]

On Tue, 2015-06-23 at 11:00AM -0700, Moritz Fischer wrote:
 Changes from v4:
 - Changed mbox cells property from 1 to 0
 - Fixed interrupt property
 
 Changes from v3:
 - Changed reg size to 0x100
 
 Changes from v2:
 - Addressed Michal's stylistic comments
 - Fixed typo in compatible string
 
 Changes from v1:
 - Added common clock framework support
 
 Changes from v0:
 - Fixed example bindings
 
 Signed-off-by: Moritz Fischer moritz.fisc...@ettus.com
 Acked-by: Michal Simek michal.si...@xilinx.com
 ---
  .../devicetree/bindings/mailbox/xilinx-mailbox.txt | 44 
 ++
  1 file changed, 44 insertions(+)
  create mode 100644 
 Documentation/devicetree/bindings/mailbox/xilinx-mailbox.txt
 
 diff --git a/Documentation/devicetree/bindings/mailbox/xilinx-mailbox.txt 
 b/Documentation/devicetree/bindings/mailbox/xilinx-mailbox.txt
 new file mode 100644
 index 000..269c026
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/mailbox/xilinx-mailbox.txt
 @@ -0,0 +1,44 @@
 +Xilinx Mailbox Driver
 +=
 +
 +Required properties:
 +- compatible   : xlnx,mailbox-2.1.
 +- reg  :  physical base address of the mailbox and length of
 +  memory mapped region.
 +- #mbox-cells  :  common mailbox binding property to identify the number
 +  of cells required for the mailbox specifier, should be 
 1

I guess this should say should be 0 now?

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

Re: [LINUX RFC V2 1/2] devicetree: Add DT bindings documentation for Zynq Ultrascale+ MPSoC GQSPI controller

[Sören Brinkmann]

On Fri, 2015-06-05 at 06:37PM +0530, Ranjit Waghmode wrote:
 Add bindings documentation for GQSPI controller driver used by
 Zynq Ultrascale+ MPSoC
 
 Signed-off-by: Ranjit Waghmode ranjit.waghm...@xilinx.com
 ---
 No changes in v2
 ---
  .../devicetree/bindings/spi/spi-zynqmp-qspi.txt| 26 
 ++
  1 file changed, 26 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.txt
 
 diff --git a/Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.txt 
 b/Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.txt
 new file mode 100644
 index 000..cec6330
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.txt
 @@ -0,0 +1,26 @@
 +Xilinx Zynq UltraScale+ MPSoC GQSPI controller Device Tree Bindings
 +---
 +
 +Required properties:
 +- compatible : Should be xlnx,zynqmp-qspi-1.0.
 +- reg: Physical base address and size of GQSPI 
 registers map.
 +- interrupts : Property with a value describing the interrupt
 +   number.
 +- interrupt-parent   : Must be core interrupt controller.
 +- clock-names: List of input clock names - ref_clk, pclk
 +   (See clock bindings for details).
 +- clocks : Clock phandles (see clock bindings for details).
 +
 +Optional properties:
 +- num-cs : Number of chip selects used.
 +
 +Example:
 + qspi: spi@ff0f {
 + compatible = xlnx,zynqmp-qspi-1.0;
 + clock-names = ref_clk, pclk;
 + clocks = misc_clk misc_clk;
 + interrupts = 0 15 4;
 + interrupt-parent = gic;
 + num-cs = 1;
 + reg = 0x0 0xff0f 0x1000 0x0 0xc000 0x800;

Please make this
  reg = 0x0 0xff0f 0x1000, 0x0 0xc000 0x800;

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

Re: [PATCH v3] ARM: zynq: DT: Use the zynq binding with macb

[Sören Brinkmann]

On Wed, 2015-05-27 at 03:00PM -0500, Nathan Sullivan wrote:
 Use the new zynq binding for macb ethernet, since it will disable half
 duplex gigabit like the Zynq TRM says to do. Also allow the compatible
 cadence gem binding that won't disable half duplex but works otherwise.
 
 Signed-off-by: Nathan Sullivan nathan.sulli...@ni.com

Assuming that the corresponding patch(es) for the driver and DT docs
went through:
Acked-by: Sören Brinkmann soren.brinkm...@xilinx.com

 ---

For future reference, please include a changelog here documenting what
changes between patch iterations.

Thanks,
Sören
--
To unsubscribe from this list: send the line unsubscribe devicetree in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] dts: Adding docs for Xilinx LogiCORE IP mailbox driver.

[Sören Brinkmann]

On Fri, 2015-05-22 at 07:43AM +0200, Michal Simek wrote:
 On 05/22/2015 01:37 AM, Moritz Fischer wrote:
  Signed-off-by: Moritz Fischer moritz.fisc...@ettus.com
  ---
   .../bindings/mailbox/xilinx-mailbox.txt | 40 
   1 file changed, 40 insertions(+)
  
 
 IRC the rule was to send binding first and then the driver.
 
  diff --git a/Documentation/devicetree/bindings/mailbox/xilinx-mailbox.txt 
  b/Documentation/devicetree/bindings/mailbox/xilinx-mailbox.txt
  new file mode 100644
  index 000..e559743
  --- /dev/null
  +++ b/Documentation/devicetree/bindings/mailbox/xilinx-mailbox.txt
  @@ -0,0 +1,40 @@
  +Xilinx Mailbox Driver
  +=
  +
  +Required properties:
  +- compatible : xlnx,mailbox-2.1.
  +- reg :physical base address of the mailbox and length of
  +   memory mapped region.
  +- #mbox-cells: Common mailbox binding property to identify the number
  +   of cells required for the mailbox specifier. Should be 1.
  +
  +Optional properties:
  +- interrupt-parent :   interrupt source phandle.
  +- interrupts : interrupt number. The interrupt specifier format
 
 please be consistent with spacing around :. It doesn't look nice.
 
  +   depends on the interrupt controller parent.
  +
  +Example:
  +   mbox: mailbox@0x4040 {
 
 remove 0x prefix here.
 
  +   compatible = xlnx,axi-mailbox-2.1;
  +   reg = 0x100 0x3c;
 
 This is weird - it should start with 4040.
 Or is your physical address 0x100?
 
 
  +   interrupt-parent = intc;
  +   interrupts = 5;
  +   #mbox-cells = 1;
  +   };
  +
  +Mailbox client
  +===
  +mboxes and the optional mbox-names (please see
  +Documentation/devicetree/bindings/mailbox/mailbox.txt for details). Each 
  value
  +of the mboxes property should contain a phandle to the mailbox controller
  +device node and second argument is the channel index. It must be 0 
  (hardware
  +support only one channel).The equivalent mbox-names property value can be
 
 .spaceThe
 
  +used to give a name to the communication channel to be used by the client 
  user.
  +
  +Example:
  +   mclient0: mclient0@0x400 {
 
 ditto.
 
  +   compatible = client-1.0;
  +   reg = 0x400 0x10;
  +   mbox-names = mbox, mbox-rx;

This seems to consume two mailboxes but...

  +   mboxes = mbox 0;

... this seems to describe only one, or am I mistaken?

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

Re: [RFC PATCH 1/2] devicetree: Add devicetree bindings documentation for ZynqMP GQSPI

[Sören Brinkmann]

On Wed, 2015-05-20 at 12:57PM +0530, Ranjit Waghmode wrote:
 Add bindings documentation for GQSPI controller driver used by
 Zynq Ultrascale+ MPSoC
 
 Signed-off-by: Ranjit Waghmode ranjit.waghm...@xilinx.com
 ---
  .../devicetree/bindings/spi/spi-zynqmp-qspi.txt| 26 
 ++
  1 file changed, 26 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.txt
 
 diff --git a/Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.txt 
 b/Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.txt
 new file mode 100644
 index 000..cec6330
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.txt
 @@ -0,0 +1,26 @@
 +Xilinx Zynq UltraScale+ MPSoC GQSPI controller Device Tree Bindings
 +---
 +
 +Required properties:
 +- compatible : Should be xlnx,zynqmp-qspi-1.0.
 +- reg: Physical base address and size of GQSPI 
 registers map.
 +- interrupts : Property with a value describing the interrupt
 +   number.
 +- interrupt-parent   : Must be core interrupt controller.
 +- clock-names: List of input clock names - ref_clk, pclk
 +   (See clock bindings for details).
 +- clocks : Clock phandles (see clock bindings for details).
 +
 +Optional properties:
 +- num-cs : Number of chip selects used.
 +
 +Example:
 + qspi: spi@ff0f {
 + compatible = xlnx,zynqmp-qspi-1.0;
 + clock-names = ref_clk, pclk;
 + clocks = misc_clk misc_clk;
 + interrupts = 0 15 4;
 + interrupt-parent = gic;
 + num-cs = 1;
 + reg = 0x0 0xff0f 0x1000 0x0 0xc000 0x800;

I find things a lot easier to read when there is something separating
the groups. Could this become:
reg = 0x0 0xff0f 0x1000, 0x0 0xc000 0x800;
?

Thanks,
Sören
--
To unsubscribe from this list: send the line unsubscribe devicetree in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 2/2] spi: Add support for Zynq Ultrascale+ MPSoC GQSPI controller

[Sören Brinkmann]

Hi Ranjit,

On Wed, 2015-05-20 at 12:57PM +0530, Ranjit Waghmode wrote:
 This patch adds support for GQSPI controller driver used by
 Zynq Ultrascale+ MPSoC
 
 Signed-off-by: Ranjit Waghmode ranjit.waghm...@xilinx.com
 ---
[...]
 +/**
 + * zynqmp_gqspi_read:For GQSPI controller read operation
 + * @xqspi:   Pointer to the zynqmp_qspi structure
 + * @offset:  Offset from where to read
 + */
 +static u32 zynqmp_gqspi_read(struct zynqmp_qspi *xqspi, u32 offset)
 +{
 + return readl_relaxed(xqspi-regs + offset);
 +}
 +
 +/**
 + * zynqmp_gqspi_write:   For GQSPI controller write operation
 + * @xqspi:   Pointer to the zynqmp_qspi structure
 + * @offset:  Offset where to write
 + * @val: Value to be written
 + */
 +static inline void zynqmp_gqspi_write(struct zynqmp_qspi *xqspi, u32 offset,
 +   u32 val)
 +{
 + writel_relaxed(val, (xqspi-regs + offset));
 +}

why are you wrapping (readl|writel)_relaxed?

[...]
 +/**
 + * zynqmp_qspi_copy_read_data:   Copy data to RX buffer
 + * @xqspi:   Pointer to the zynqmp_qspi structure
 + * @data:The 32 bit variable where data is stored
 + * @size:Number of bytes to be copied from data to RX buffer
 + */
 +static void zynqmp_qspi_copy_read_data(struct zynqmp_qspi *xqspi,
 +u32 data, u8 size)
 +{
 + memcpy(xqspi-rxbuf, ((u8 *) data), size);

Is this cast really needed? IIRC, memcpy takes (void *). The outer
parentheses for that argument are not needed.

 + xqspi-rxbuf += size;
 + xqspi-bytes_to_receive -= size;
 +}
 +
 +/**
 + * zynqmp_prepare_transfer_hardware: Prepares hardware for transfer.
 + * @master:  Pointer to the spi_master structure which provides
 + *   information about the controller.
 + *
 + * This function enables SPI master controller.
 + *
 + * Return:   Always 0
 + */
 +static int zynqmp_prepare_transfer_hardware(struct spi_master *master)
 +{
 + struct zynqmp_qspi *xqspi = spi_master_get_devdata(master);
 +
 + clk_enable(xqspi-refclk);
 + clk_enable(xqspi-pclk);

Did you consider using runtime_pm? Then this would just bit
pm_runtime_get_sync(...)

 + zynqmp_gqspi_write(xqspi, GQSPI_EN_OFST, GQSPI_EN_MASK);
 + return 0;
 +}
 +
 +/**
 + * zynqmp_unprepare_transfer_hardware:   Relaxes hardware after transfer
 + * @master:  Pointer to the spi_master structure which provides
 + *   information about the controller.
 + *
 + * This function disables the SPI master controller.
 + *
 + * Return:   Always 0
 + */
 +static int zynqmp_unprepare_transfer_hardware(struct spi_master *master)
 +{
 + struct zynqmp_qspi *xqspi = spi_master_get_devdata(master);
 +
 + zynqmp_gqspi_write(xqspi, GQSPI_EN_OFST, 0x0);
 + clk_disable(xqspi-refclk);
 + clk_disable(xqspi-pclk);

and this would become pm_runtime_put()

[...]
 +/**
 + * zynqmp_qspi_filltxfifo:   Fills the TX FIFO as long as there is room in
 + *   the FIFO or the bytes required to be
 + *   transmitted.
 + * @xqspi:   Pointer to the zynqmp_qspi structure
 + * @size:Number of bytes to be copied from TX buffer to TX FIFO
 + */
 +static void zynqmp_qspi_filltxfifo(struct zynqmp_qspi *xqspi, int size)
 +{
 + u32 count = 0;
 +
 + while ((xqspi-bytes_to_transfer  0)  (count  size)) {
 + writel(*((u32 *) xqspi-txbuf), xqspi-regs + GQSPI_TXD_OFST);

Here the writel wrapper is not used. Is there a reason, then it would
probably  deserve a comment.

[...]
 +/**
 + * zynqmp_process_dma_irq:   Handler for DMA done interrupt of QSPI
 + *   controller
 + * @xqspi:   zynqmp_qspi instance pointer
 + *
 + * This function handles DMA interrupt only.
 + */
 +static void zynqmp_process_dma_irq(struct zynqmp_qspi *xqspi)
 +{
 + u32 config_reg, genfifoentry;
 +
 + dma_unmap_single(xqspi-dev, xqspi-dma_addr,
 + xqspi-dma_rx_bytes, DMA_FROM_DEVICE);
 + xqspi-rxbuf += xqspi-dma_rx_bytes;
 + xqspi-bytes_to_receive -= xqspi-dma_rx_bytes;
 + xqspi-dma_rx_bytes = 0;
 +
 + /* Disabling the DMA interrupts */
 + writel(GQSPI_QSPIDMA_DST_I_EN_DONE_MASK,
 + xqspi-regs + GQSPI_QSPIDMA_DST_I_DIS_OFST);
 +
 + if (xqspi-bytes_to_receive  0) {
 + /* Switch to IO mode,for remaining bytes to receive */
 + config_reg = readl(xqspi-regs + GQSPI_CONFIG_OFST);
 + config_reg = ~GQSPI_CFG_MODE_EN_MASK;
 + writel(config_reg, xqspi-regs + GQSPI_CONFIG_OFST);
 +
 + /* Initiate the transfer of remaining bytes */
 + genfifoentry = xqspi-genfifoentry;
 + genfifoentry |= xqspi-bytes_to_receive;
 + writel(genfifoentry,
 + xqspi-regs + GQSPI_GEN_FIFO_OFST);
 +
 + /* Dummy generic FIFO entry */
 + writel(0x0, xqspi-regs + GQSPI_GEN_FIFO_OFST);

not using wrapper?

 +
 + /* 

Re: [PATCH] clocksource: arm_global_timer: Detect if gt is usable with CPU_FREQ

[Sören Brinkmann]

On Tue, 2015-04-14 at 08:41AM +0100, Srinivas Kandagatla wrote:
 +Adding Pete and Maxime
 
 Hi Ola,
 Thankyou for sending the patch,
 
 I like the Idea, but I have some specific concerns which would break
 existing SOCs.
 
 On 13/04/15 18:37, Ola Jeppsson wrote:
 Some Cortex A9 CPU:s (e.g. zynq) have the tick tied to the CPU
 frequency. On those CPU:s we cannot use the global-timer as a reliable
 clocksource with CPU frequency scaling enabled since this is not
 currently taken into account by the driver.
 
 Add a tied-to-cpu-freq boolean to the global-timer dt node indicate
 this condition.
 
 When the global-timer register function sees this property return
 immediately and don't register the clocksource.
 
 Signed-off-by: Ola Jeppsson o...@adapteva.com
 ---
   Documentation/devicetree/bindings/arm/global_timer.txt | 4 
   drivers/clocksource/arm_global_timer.c | 7 +++
   2 files changed, 11 insertions(+)
 
 diff --git a/Documentation/devicetree/bindings/arm/global_timer.txt 
 b/Documentation/devicetree/bindings/arm/global_timer.txt
 index bdae3a818793..465e02c17b5b 100644
 --- a/Documentation/devicetree/bindings/arm/global_timer.txt
 +++ b/Documentation/devicetree/bindings/arm/global_timer.txt
 @@ -17,6 +17,10 @@
 
   - clocks : Should be phandle to a clock.
 
 +** Timer node optional properties:
 +
 +- tied-to-cpu-freq : indicates that the timer scales with the CPU frequency.
 +
   Example:
 
  timer@2c000600 {
 diff --git a/drivers/clocksource/arm_global_timer.c 
 b/drivers/clocksource/arm_global_timer.c
 index e6833771a716..8913ebda3f09 100644
 --- a/drivers/clocksource/arm_global_timer.c
 +++ b/drivers/clocksource/arm_global_timer.c
 @@ -268,6 +268,13 @@ static void __init global_timer_of_register(struct 
 device_node *np)
  return;
  }
 
 +#ifdef CONFIG_CPU_FREQ
 +if (of_property_read_bool(np, tied-to-cpu-freq)) {
 +pr_warn(global-timer: tied to cpu frequency, not supported 
 with scaling\n);
 +return;
 +}
 +#endif
 +
 
 This patch would not let the SOC like STiH415/416 or zynq with
 tied-to-cpu-freq property to boot with multi_v7_defconfig. Which is not
 correct thing to do, as STi SOC's do not use cpufreq driver however the tick
 is tied to this clocksource.

For Zynq, it should be OK, since we have the cadence_ttc as alternative
clocksource. Though, I have to admit not having tested this patch.

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

Re: [PATCH] dma: Add Xilinx ZDMA device tree Binding Documentation

[Sören Brinkmann]

On Tue, 2015-03-10 at 07:46PM +0530, Punnaiah Choudary Kalluri wrote:
 Device-tree binding documentation for Xilinx ZDMA Engine
 
 Signed-off-by: Punnaiah Choudary Kalluri punn...@xilinx.com
 ---
  .../devicetree/bindings/dma/xilinx/zdma.txt|   76 
 
  1 files changed, 76 insertions(+), 0 deletions(-)
  create mode 100644 Documentation/devicetree/bindings/dma/xilinx/zdma.txt
 
 diff --git a/Documentation/devicetree/bindings/dma/xilinx/zdma.txt 
 b/Documentation/devicetree/bindings/dma/xilinx/zdma.txt
 new file mode 100644
 index 000..399a3bc
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/dma/xilinx/zdma.txt
 @@ -0,0 +1,76 @@
 +Xilinx ZDMA engine, it does support memory to memory transfers,
 +memory to device and device to memory transfers. It also has flow
 +control and rate control support for slave/peripheral dma access.
 +
 +Xilinx ZynqMP has two instances of general purpose DMA(ZDMA).
 +one is located in FPD(full power domain) and other is located in
 +LPD(low power domain).
 +
 +ZDMA instance located in FPD is referred as FPDMA and instance located
 +in LPD is referred as LPDMA.
 +
 +FPDMA is configured with 8 DMA channels and AXI bus width is 128 byte.
 +LPDMA is configured with 8 DMA channels and AXI bus width is 64 byte.

All these implementation details are good background information, but
shouldn't be part of the binding.

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

Re: [PATCH v2] ARM: dts: zynq: Add OCM node

[Sören Brinkmann]

On Thu, 2015-02-12 at 12:01PM +0100, Michal Simek wrote:
 On 02/12/2015 11:54 AM, Mark Rutland wrote:
  On Thu, Feb 12, 2015 at 10:42:47AM +, Michal Simek wrote:
  Add OCM node for all zynq boards. OCM location
  can changed but for all current boards this
  is the location where OCM is.`
 
  Signed-off-by: Michal Simek michal.si...@xilinx.com
  ---
 
  Changes in v2:
  - Move node to board file suggested by Mark
 
  This patch is done based on discussion here.
  https://lkml.org/lkml/2014/12/1/396
 
  Mark: I expect you won't like amba bus reference or
  am I wrong?
  
  I'm fine with dropping things onto a bus in this way. If we're happy to
  do it for other nodes I don't see why busses should be special.
 
 Wonderful. I will give people some time to comment this style.

Given that the location is discoverable, wouldn't it make sense to let
'reg' point to the ctrl/cfg registers in the SLCR and let the driver
handle the whereabouts of the OCM location? (but I guess this is going
in circles now, such a proposal was on the table at some point, IIRC).
But I'd prefer:
memory-controller@0xfffc { /* the address here would of course not 
match all configurations */
interrupts = ...;
syscon = slcr;
};

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

Re: [PATCH v2] ARM: dts: zynq: Add OCM node

[Sören Brinkmann]

On Thu, 2015-02-12 at 03:07PM +, Mark Rutland wrote:
 On Thu, Feb 12, 2015 at 02:58:36PM +, Sören Brinkmann wrote:
  On Thu, 2015-02-12 at 12:01PM +0100, Michal Simek wrote:
   On 02/12/2015 11:54 AM, Mark Rutland wrote:
On Thu, Feb 12, 2015 at 10:42:47AM +, Michal Simek wrote:
Add OCM node for all zynq boards. OCM location
can changed but for all current boards this
is the location where OCM is.`
   
Signed-off-by: Michal Simek michal.si...@xilinx.com
---
   
Changes in v2:
- Move node to board file suggested by Mark
   
This patch is done based on discussion here.
https://lkml.org/lkml/2014/12/1/396
   
Mark: I expect you won't like amba bus reference or
am I wrong?

I'm fine with dropping things onto a bus in this way. If we're happy to
do it for other nodes I don't see why busses should be special.
   
   Wonderful. I will give people some time to comment this style.
  
  Given that the location is discoverable, wouldn't it make sense to let
  'reg' point to the ctrl/cfg registers in the SLCR and let the driver
  handle the whereabouts of the OCM location? (but I guess this is going
  in circles now, such a proposal was on the table at some point, IIRC).
  But I'd prefer:
  memory-controller@0xfffc { /* the address here would of course not 
  match all configurations */
 
 I'd really prefer that we keep the unit-address and reg consistent.
 
 Given that the address may change on a per-board basis, it simply has to
 live in the board file.

It's not a per-board but rather per use-case configuration.

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

Re: [PATCH 1/2] ARM: zynq: Remove bogus value from 'bias-disable' property

[Sören Brinkmann]

On Wed, 2015-01-28 at 03:21PM +0100, Michal Simek wrote:
 On 01/27/2015 01:38 AM, Andreas Färber wrote:
  Am 26.01.2015 um 20:49 schrieb Soren Brinkmann:
  In one pinctrl node, a 'bias-disable' property is erroneously assigned a
  value.
 
  Fixes: ARM: zynq: DT: Add pinctrl information
  Signed-off-by: Soren Brinkmann soren.brinkm...@xilinx.com
  
  Reviewed-by: Andreas Färber afaer...@suse.de
  
  Andreas
  
 
 Applied both.

Thanks. Given how the branch looks now, you could squash 'ARM: zynq: DT:
Remove bogus value from 'bias-disable' property' and 'ARM: zynq: DT: Add
pinctrl information to USB nodes' into 'ARM: zynq: DT: Add pinctrl
information'. Then we have it all in one patch and spare everybody the
intermittent, partly broken commits.

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

Re: [PATCH] usb: phy-generic: Don't fail on missing gpio reset

[Sören Brinkmann]

On Tue, 2015-01-27 at 09:20AM -0600, Felipe Balbi wrote:
 On Mon, Jan 26, 2015 at 05:45:29PM -0800, Soren Brinkmann wrote:
  A reset through a GPIO is optional. Don't fail probing when it is
  missing.
  
  Reported-by: Andreas Färber afaer...@suse.de
  Signed-off-by: Soren Brinkmann soren.brinkm...@xilinx.com
  ---
  Hi Andreas,
  
  does this do the trick?
  
  Thanks,
  Sören
  
   drivers/usb/phy/phy-generic.c | 6 ++
   1 file changed, 2 insertions(+), 4 deletions(-)
  
  diff --git a/drivers/usb/phy/phy-generic.c b/drivers/usb/phy/phy-generic.c
  index dd05254241fb..a73d4c738f0b 100644
  --- a/drivers/usb/phy/phy-generic.c
  +++ b/drivers/usb/phy/phy-generic.c
  @@ -241,10 +241,8 @@ int usb_phy_gen_create_phy(struct device *dev, struct 
  usb_phy_generic *nop,
   
  if (err == -EPROBE_DEFER)
  return -EPROBE_DEFER;
  -   if (err) {
  -   dev_err(dev, Error requesting RESET GPIO\n);
  -   return err;
  -   }
  +   if (err)
  +   nop-gpiod_reset = NULL;
 
 there's a better patch to use gpiod_get_optional(), instead.

Great, apparently that wasn't in linux-next yesterday. I'll give it a
shot once it arrives there.

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

Re: [PATCH v3] ARM: zynq: DT: Add USB to device tree

[Sören Brinkmann]

On Mon, 2015-01-26 at 05:21PM +0100, Andreas Färber wrote:
 Am 26.01.2015 um 16:50 schrieb Sören Brinkmann:
  On Mon, 2015-01-26 at 10:35AM +0100, Andreas Färber wrote:
  Am 26.01.2015 um 09:33 schrieb Andreas Färber:
  Am 26.01.2015 um 09:23 schrieb Michal Simek:
  On 01/26/2015 09:19 AM, Andreas Färber wrote:
  And if I apply it to my -next based tree, adding corresponding nodes to
  zynq-parallella.dts, I get repeatedly:
 
  [  +0,012242] ci_hdrc ci_hdrc.0: no of_node; not parsing pinctrl DT
  [  +0,000157] ci_hdrc ci_hdrc.0: ChipIdea HDRC found, lpm: 0; cap:
  f090e100 op: f090e140
  [  +0,81] platform ci_hdrc.0: Driver ci_hdrc requests probe deferral
  [  +0,005360] ci_hdrc ci_hdrc.1: no of_node; not parsing pinctrl DT
  [  +0,000120] ci_hdrc ci_hdrc.1: ChipIdea HDRC found, lpm: 0; cap:
  f0910100 op: f0910140
  [  +0,001810] platform ci_hdrc.1: Driver ci_hdrc requests probe deferral
 
  Am I missing any other patches or config options?
  (I do notice that the pinctrl v3 patch that got merged has a trivial bug
  for usb0, for which I'll send a patch later on.)
 
  Why is it deferred? Is it because of pinmuxing stuff?
 
  No, happened without as well.
 
  Looking at a different place in dmesg, I spot this:
 
  [  +0,003988] usb_phy_generic phy0: GPIO lookup for consumer reset-gpios
  [  +0,12] usb_phy_generic phy0: using device tree for GPIO lookup
  [  +0,15] of_get_named_gpiod_flags: can't parse 'reset-gpios-gpios'
  property
   of node '/phy0[0]'
  [  +0,13] of_get_named_gpiod_flags: can't parse 'reset-gpios-gpio'
  property
  of node '/phy0[0]'
  [  +0,10] usb_phy_generic phy0: using lookup tables for GPIO lookup
  [  +0,000153] usb_phy_generic phy0: lookup for GPIO reset-gpios failed
  [  +0,12] usb_phy_generic phy0: Error requesting RESET GPIO
  [  +0,004360] usb_phy_generic: probe of phy0 failed with error -2
  [  +0,004991] usb_phy_generic phy1: GPIO lookup for consumer reset-gpios
  [  +0,12] usb_phy_generic phy1: using device tree for GPIO lookup
  [  +0,13] of_get_named_gpiod_flags: can't parse 'reset-gpios-gpios'
  property
   of node '/phy1[0]'
  [  +0,13] of_get_named_gpiod_flags: can't parse 'reset-gpios-gpio'
  property of node '/phy1[0]'
  [  +0,10] usb_phy_generic phy1: using lookup tables for GPIO lookup
  [  +0,12] usb_phy_generic phy1: lookup for GPIO reset-gpios failed
  [  +0,11] usb_phy_generic phy1: Error requesting RESET GPIO
  [  +0,004337] usb_phy_generic: probe of phy1 failed with error -2
 
  So, I guess the chipidea driver is deferring because the phys want a
  property that neither me nor you are specifying? Would that be the two
  MDIO pins 52 and 53 that would need to be specified?
 
  Erm, scratch that last question - wrong PHY. Trying it resolved the
  above phy errors but not the original problem. And so does an empty one:
 
  @@ -99,11 +100,13 @@
 
  usb_phy0: phy0 {
  compatible = usb-nop-xceiv;
  +   reset-gpios = ;
  #phy-cells = 0;
  };
 
  usb_phy1: phy1 {
  compatible = usb-nop-xceiv;
  +   reset-gpios = ;
  #phy-cells = 0;
  };
   };
 
  In my manuals and notes I can't find any GPIO being used as reset for
  the USB PHYs. Any thoughts appreciated.
  
  Such a connection is optional. The platform might rely on its reset
  circuit, though it might not work for warm reboots.
  I haven't looked at parallela docs, but if there is a schematic
  available, that should tell you if/what is connected to the PHY reset
  pin.
 
 I do have the schematic, and the way I read it, only the on-board reset
 button resets the PHYs.
 
 Yet it looks as if usb-nop-xceiv insists on a reset-gpios above, no?
 Does it work on your boards with linux-next?

I haven't re-tested it since I submitted the patches, but at that time
it worked. But I also didn't test USB with the pinctrl patches together.
I'll do some testing later today.

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

Re: [PATCH v3] ARM: zynq: DT: Add USB to device tree

[Sören Brinkmann]

On Mon, 2015-01-26 at 10:35AM +0100, Andreas Färber wrote:
 Am 26.01.2015 um 09:33 schrieb Andreas Färber:
  Am 26.01.2015 um 09:23 schrieb Michal Simek:
  On 01/26/2015 09:19 AM, Andreas Färber wrote:
  And if I apply it to my -next based tree, adding corresponding nodes to
  zynq-parallella.dts, I get repeatedly:
 
  [  +0,012242] ci_hdrc ci_hdrc.0: no of_node; not parsing pinctrl DT
  [  +0,000157] ci_hdrc ci_hdrc.0: ChipIdea HDRC found, lpm: 0; cap:
  f090e100 op: f090e140
  [  +0,81] platform ci_hdrc.0: Driver ci_hdrc requests probe deferral
  [  +0,005360] ci_hdrc ci_hdrc.1: no of_node; not parsing pinctrl DT
  [  +0,000120] ci_hdrc ci_hdrc.1: ChipIdea HDRC found, lpm: 0; cap:
  f0910100 op: f0910140
  [  +0,001810] platform ci_hdrc.1: Driver ci_hdrc requests probe deferral
 
  Am I missing any other patches or config options?
  (I do notice that the pinctrl v3 patch that got merged has a trivial bug
  for usb0, for which I'll send a patch later on.)
 
  Why is it deferred? Is it because of pinmuxing stuff?
  
  No, happened without as well.
  
  Looking at a different place in dmesg, I spot this:
  
  [  +0,003988] usb_phy_generic phy0: GPIO lookup for consumer reset-gpios
  [  +0,12] usb_phy_generic phy0: using device tree for GPIO lookup
  [  +0,15] of_get_named_gpiod_flags: can't parse 'reset-gpios-gpios'
  property
   of node '/phy0[0]'
  [  +0,13] of_get_named_gpiod_flags: can't parse 'reset-gpios-gpio'
  property
  of node '/phy0[0]'
  [  +0,10] usb_phy_generic phy0: using lookup tables for GPIO lookup
  [  +0,000153] usb_phy_generic phy0: lookup for GPIO reset-gpios failed
  [  +0,12] usb_phy_generic phy0: Error requesting RESET GPIO
  [  +0,004360] usb_phy_generic: probe of phy0 failed with error -2
  [  +0,004991] usb_phy_generic phy1: GPIO lookup for consumer reset-gpios
  [  +0,12] usb_phy_generic phy1: using device tree for GPIO lookup
  [  +0,13] of_get_named_gpiod_flags: can't parse 'reset-gpios-gpios'
  property
   of node '/phy1[0]'
  [  +0,13] of_get_named_gpiod_flags: can't parse 'reset-gpios-gpio'
  property of node '/phy1[0]'
  [  +0,10] usb_phy_generic phy1: using lookup tables for GPIO lookup
  [  +0,12] usb_phy_generic phy1: lookup for GPIO reset-gpios failed
  [  +0,11] usb_phy_generic phy1: Error requesting RESET GPIO
  [  +0,004337] usb_phy_generic: probe of phy1 failed with error -2
  
  So, I guess the chipidea driver is deferring because the phys want a
  property that neither me nor you are specifying? Would that be the two
  MDIO pins 52 and 53 that would need to be specified?
 
 Erm, scratch that last question - wrong PHY. Trying it resolved the
 above phy errors but not the original problem. And so does an empty one:
 
 @@ -99,11 +100,13 @@
 
 usb_phy0: phy0 {
 compatible = usb-nop-xceiv;
 +   reset-gpios = ;
 #phy-cells = 0;
 };
 
 usb_phy1: phy1 {
 compatible = usb-nop-xceiv;
 +   reset-gpios = ;
 #phy-cells = 0;
 };
  };
 
 In my manuals and notes I can't find any GPIO being used as reset for
 the USB PHYs. Any thoughts appreciated.

Such a connection is optional. The platform might rely on its reset
circuit, though it might not work for warm reboots.
I haven't looked at parallela docs, but if there is a schematic
available, that should tell you if/what is connected to the PHY reset
pin.

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

Re: [PATCH v3] ARM: zynq: DT: Add USB to device tree

[Sören Brinkmann]

On Mon, 2015-01-26 at 08:23AM -0800, Sören Brinkmann wrote:
 On Mon, 2015-01-26 at 05:21PM +0100, Andreas Färber wrote:
  Am 26.01.2015 um 16:50 schrieb Sören Brinkmann:
   On Mon, 2015-01-26 at 10:35AM +0100, Andreas Färber wrote:
   Am 26.01.2015 um 09:33 schrieb Andreas Färber:
   Am 26.01.2015 um 09:23 schrieb Michal Simek:
   On 01/26/2015 09:19 AM, Andreas Färber wrote:
   And if I apply it to my -next based tree, adding corresponding nodes 
   to
   zynq-parallella.dts, I get repeatedly:
  
   [  +0,012242] ci_hdrc ci_hdrc.0: no of_node; not parsing pinctrl DT
   [  +0,000157] ci_hdrc ci_hdrc.0: ChipIdea HDRC found, lpm: 0; cap:
   f090e100 op: f090e140
   [  +0,81] platform ci_hdrc.0: Driver ci_hdrc requests probe 
   deferral
   [  +0,005360] ci_hdrc ci_hdrc.1: no of_node; not parsing pinctrl DT
   [  +0,000120] ci_hdrc ci_hdrc.1: ChipIdea HDRC found, lpm: 0; cap:
   f0910100 op: f0910140
   [  +0,001810] platform ci_hdrc.1: Driver ci_hdrc requests probe 
   deferral
  
   Am I missing any other patches or config options?
   (I do notice that the pinctrl v3 patch that got merged has a trivial 
   bug
   for usb0, for which I'll send a patch later on.)
  
   Why is it deferred? Is it because of pinmuxing stuff?
  
   No, happened without as well.
  
   Looking at a different place in dmesg, I spot this:
  
   [  +0,003988] usb_phy_generic phy0: GPIO lookup for consumer reset-gpios
   [  +0,12] usb_phy_generic phy0: using device tree for GPIO lookup
   [  +0,15] of_get_named_gpiod_flags: can't parse 'reset-gpios-gpios'
   property
of node '/phy0[0]'
   [  +0,13] of_get_named_gpiod_flags: can't parse 'reset-gpios-gpio'
   property
   of node '/phy0[0]'
   [  +0,10] usb_phy_generic phy0: using lookup tables for GPIO lookup
   [  +0,000153] usb_phy_generic phy0: lookup for GPIO reset-gpios failed
   [  +0,12] usb_phy_generic phy0: Error requesting RESET GPIO
   [  +0,004360] usb_phy_generic: probe of phy0 failed with error -2
   [  +0,004991] usb_phy_generic phy1: GPIO lookup for consumer reset-gpios
   [  +0,12] usb_phy_generic phy1: using device tree for GPIO lookup
   [  +0,13] of_get_named_gpiod_flags: can't parse 'reset-gpios-gpios'
   property
of node '/phy1[0]'
   [  +0,13] of_get_named_gpiod_flags: can't parse 'reset-gpios-gpio'
   property of node '/phy1[0]'
   [  +0,10] usb_phy_generic phy1: using lookup tables for GPIO lookup
   [  +0,12] usb_phy_generic phy1: lookup for GPIO reset-gpios failed
   [  +0,11] usb_phy_generic phy1: Error requesting RESET GPIO
   [  +0,004337] usb_phy_generic: probe of phy1 failed with error -2
  
   So, I guess the chipidea driver is deferring because the phys want a
   property that neither me nor you are specifying? Would that be the two
   MDIO pins 52 and 53 that would need to be specified?
  
   Erm, scratch that last question - wrong PHY. Trying it resolved the
   above phy errors but not the original problem. And so does an empty one:
  
   @@ -99,11 +100,13 @@
  
   usb_phy0: phy0 {
   compatible = usb-nop-xceiv;
   +   reset-gpios = ;
   #phy-cells = 0;
   };
  
   usb_phy1: phy1 {
   compatible = usb-nop-xceiv;
   +   reset-gpios = ;
   #phy-cells = 0;
   };
};
  
   In my manuals and notes I can't find any GPIO being used as reset for
   the USB PHYs. Any thoughts appreciated.
   
   Such a connection is optional. The platform might rely on its reset
   circuit, though it might not work for warm reboots.
   I haven't looked at parallela docs, but if there is a schematic
   available, that should tell you if/what is connected to the PHY reset
   pin.
  
  I do have the schematic, and the way I read it, only the on-board reset
  button resets the PHYs.
  
  Yet it looks as if usb-nop-xceiv insists on a reset-gpios above, no?
  Does it work on your boards with linux-next?
 
 I haven't re-tested it since I submitted the patches, but at that time
 it worked. But I also didn't test USB with the pinctrl patches together.
 I'll do some testing later today.

So, just did a test. I took all the pinctrl stuff and this patch and ran
it on a zc702. I plugged in a thumb drive and that worked just fine. So,
basically this patch could go in, despite missing pinctrl properties.

Michal: How do you wanna handle this? Could you create a branch that has
all the DT updates I can base stuff on, please? We could either take the
pinctrl patches and this patch and add another one adding the pinctrl
properties to the USB nodes. Or leave this patch out for now and revise
it on top of the pinctrl patches.

Thanks,
Sören
--
To unsubscribe from this list: send the line unsubscribe devicetree in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] ARM: zynq: DT: Add USB to device tree

[Sören Brinkmann]

On Tue, 2015-01-27 at 02:14AM +0100, Andreas Färber wrote:
 + linux-gpio, linux-usb, Felipe Balbi
 
 Am 26.01.2015 um 19:44 schrieb Sören Brinkmann:
  On Mon, 2015-01-26 at 08:23AM -0800, Sören Brinkmann wrote:
  On Mon, 2015-01-26 at 05:21PM +0100, Andreas Färber wrote:
  Am 26.01.2015 um 16:50 schrieb Sören Brinkmann:
  On Mon, 2015-01-26 at 10:35AM +0100, Andreas Färber wrote:
  Am 26.01.2015 um 09:33 schrieb Andreas Färber:
  Am 26.01.2015 um 09:23 schrieb Michal Simek:
  On 01/26/2015 09:19 AM, Andreas Färber wrote:
  And if I apply it to my -next based tree, adding corresponding nodes 
  to
  zynq-parallella.dts, I get repeatedly:
 
  [  +0,012242] ci_hdrc ci_hdrc.0: no of_node; not parsing pinctrl DT
  [  +0,000157] ci_hdrc ci_hdrc.0: ChipIdea HDRC found, lpm: 0; cap:
  f090e100 op: f090e140
  [  +0,81] platform ci_hdrc.0: Driver ci_hdrc requests probe 
  deferral
  [  +0,005360] ci_hdrc ci_hdrc.1: no of_node; not parsing pinctrl DT
  [  +0,000120] ci_hdrc ci_hdrc.1: ChipIdea HDRC found, lpm: 0; cap:
  f0910100 op: f0910140
  [  +0,001810] platform ci_hdrc.1: Driver ci_hdrc requests probe 
  deferral
 
  Am I missing any other patches or config options?
 [...]
  Why is it deferred? Is it because of pinmuxing stuff?
 
  No, happened without as well.
 
  Looking at a different place in dmesg, I spot this:
 
  [  +0,003988] usb_phy_generic phy0: GPIO lookup for consumer 
  reset-gpios
  [  +0,12] usb_phy_generic phy0: using device tree for GPIO lookup
  [  +0,15] of_get_named_gpiod_flags: can't parse 'reset-gpios-gpios'
  property
   of node '/phy0[0]'
  [  +0,13] of_get_named_gpiod_flags: can't parse 'reset-gpios-gpio'
  property
  of node '/phy0[0]'
  [  +0,10] usb_phy_generic phy0: using lookup tables for GPIO lookup
  [  +0,000153] usb_phy_generic phy0: lookup for GPIO reset-gpios failed
  [  +0,12] usb_phy_generic phy0: Error requesting RESET GPIO
  [  +0,004360] usb_phy_generic: probe of phy0 failed with error -2
  [  +0,004991] usb_phy_generic phy1: GPIO lookup for consumer 
  reset-gpios
  [  +0,12] usb_phy_generic phy1: using device tree for GPIO lookup
  [  +0,13] of_get_named_gpiod_flags: can't parse 'reset-gpios-gpios'
  property
   of node '/phy1[0]'
  [  +0,13] of_get_named_gpiod_flags: can't parse 'reset-gpios-gpio'
  property of node '/phy1[0]'
  [  +0,10] usb_phy_generic phy1: using lookup tables for GPIO lookup
  [  +0,12] usb_phy_generic phy1: lookup for GPIO reset-gpios failed
  [  +0,11] usb_phy_generic phy1: Error requesting RESET GPIO
  [  +0,004337] usb_phy_generic: probe of phy1 failed with error -2
 
  So, I guess the chipidea driver is deferring because the phys want a
  property that neither me nor you are specifying? [...]
 [...]
  In my manuals and notes I can't find any GPIO being used as reset for
  the USB PHYs. Any thoughts appreciated.
 
  Such a connection is optional. The platform might rely on its reset
  circuit, though it might not work for warm reboots.
  I haven't looked at parallela docs, but if there is a schematic
  available, that should tell you if/what is connected to the PHY reset
  pin.
 
  I do have the schematic, and the way I read it, only the on-board reset
  button resets the PHYs.
 
  Yet it looks as if usb-nop-xceiv insists on a reset-gpios above, no?
  Does it work on your boards with linux-next?
 
  I haven't re-tested it since I submitted the patches, but at that time
  it worked. But I also didn't test USB with the pinctrl patches together.
  I'll do some testing later today.
  
  So, just did a test. I took all the pinctrl stuff and this patch and ran
  it on a zc702. I plugged in a thumb drive and that worked just fine. So,
  basically this patch could go in, despite missing pinctrl properties.
 
 For my board I needed the following workaround:
 
 diff --git a/drivers/usb/phy/phy-generic.c b/drivers/usb/phy/phy-generic.c
 index dd05254241fb..96c7c36e22a6 100644
 --- a/drivers/usb/phy/phy-generic.c
 +++ b/drivers/usb/phy/phy-generic.c
 @@ -218,13 +218,13 @@ int usb_phy_gen_create_phy(struct device *dev,
 struct usb_phy_generic *nop,
 clk_rate = 0;
 
 needs_vcc = of_property_read_bool(node, vcc-supply);
 -   nop-gpiod_reset = devm_gpiod_get(dev, reset-gpios);
 +   /*nop-gpiod_reset = devm_gpiod_get(dev, reset-gpios);
 err = PTR_ERR(nop-gpiod_reset);
 if (!err) {
 nop-gpiod_vbus = devm_gpiod_get(dev,
 
 vbus-detect-gpio);
 err = PTR_ERR(nop-gpiod_vbus);
 -   }
 +   }*/
 } else if (pdata) {
 type = pdata-type;
 clk_rate = pdata-clk_rate;
 
 [  +0,004738] usb_phy_generic phy0: Looking up vcc-supply from device tree
 [  +0,18] usb_phy_generic phy0: Looking up vcc-supply property in
 node /phy0 failed
 [  +0,11] phy0 supply vcc not found, using dummy regulator
 [  +0,004844

Re: [PATCH v4 6/7] ARM: zynq: DT: Add pinctrl information

[Sören Brinkmann]

On Tue, 2015-01-27 at 12:57AM +0100, Andreas Färber wrote:
 Am 09.01.2015 um 16:43 schrieb Soren Brinkmann:
  Add pinctrl descriptions to the zc702 and zc706 device trees.
  
  Signed-off-by: Soren Brinkmann soren.brinkm...@xilinx.com
  Tested-by: Andreas Färber afaer...@suse.de
  ---
  Changes since v1:
   - remove 'pinctrl-' prefix for pinctrl sub-nodes
   - separate config and mux nodes
  
  Changes since RFC v2:
   - add pinconf properties to zc702 mdio node
   - remove arguments from bias-related props
  
  Changes since RFC v1:
   - separate DT changes into their own patch
  ---
   arch/arm/boot/dts/zynq-7000.dtsi |   8 +-
   arch/arm/boot/dts/zynq-zc702.dts | 181 
  +++
   arch/arm/boot/dts/zynq-zc706.dts | 152 
   3 files changed, 340 insertions(+), 1 deletion(-)
 [...]
  diff --git a/arch/arm/boot/dts/zynq-zc702.dts 
  b/arch/arm/boot/dts/zynq-zc702.dts
  index 280f02dd4ddc..4995412f116f 100644
  --- a/arch/arm/boot/dts/zynq-zc702.dts
  +++ b/arch/arm/boot/dts/zynq-zc702.dts
 [...]
  @@ -50,15 +52,24 @@
  status = okay;
  phy-mode = rgmii-id;
  phy-handle = ethernet_phy;
  +   pinctrl-names = default;
  +   pinctrl-0 = pinctrl_gem0_default;
   
  ethernet_phy: ethernet-phy@7 {
  reg = 7;
  };
   };
   
  +gpio0 {
  +   pinctrl-names = default;
  +   pinctrl-0 = pinctrl_gpio0_default;
 
 On linux-next the equivalent no longer works for the Parallella, with
 gpio failing to probe.
 
 If I move these two properties to the leds node (for which I am
 configuring gpio 7) then I get a heartbeat as before.

I think for USB I have a fix (see other email with patch), but LEDs seem
to be broken too. On my zc702 I get:
   of_get_named_gpiod_flags: parsed 'gpios' property of node '/leds/ds23[0]' - 
status (-517)

517 is probe deferral. Looks like the LED driver needs to learn to defer
probing when the GPIO driver isn't available yet.

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

Re: [PATCH v4 6/7] ARM: zynq: DT: Add pinctrl information

[Sören Brinkmann]

On Tue, 2015-01-27 at 12:57AM +0100, Andreas Färber wrote:
 Am 09.01.2015 um 16:43 schrieb Soren Brinkmann:
  Add pinctrl descriptions to the zc702 and zc706 device trees.
  
  Signed-off-by: Soren Brinkmann soren.brinkm...@xilinx.com
  Tested-by: Andreas Färber afaer...@suse.de
  ---
  Changes since v1:
   - remove 'pinctrl-' prefix for pinctrl sub-nodes
   - separate config and mux nodes
  
  Changes since RFC v2:
   - add pinconf properties to zc702 mdio node
   - remove arguments from bias-related props
  
  Changes since RFC v1:
   - separate DT changes into their own patch
  ---
   arch/arm/boot/dts/zynq-7000.dtsi |   8 +-
   arch/arm/boot/dts/zynq-zc702.dts | 181 
  +++
   arch/arm/boot/dts/zynq-zc706.dts | 152 
   3 files changed, 340 insertions(+), 1 deletion(-)
 [...]
  diff --git a/arch/arm/boot/dts/zynq-zc702.dts 
  b/arch/arm/boot/dts/zynq-zc702.dts
  index 280f02dd4ddc..4995412f116f 100644
  --- a/arch/arm/boot/dts/zynq-zc702.dts
  +++ b/arch/arm/boot/dts/zynq-zc702.dts
 [...]
  @@ -50,15 +52,24 @@
  status = okay;
  phy-mode = rgmii-id;
  phy-handle = ethernet_phy;
  +   pinctrl-names = default;
  +   pinctrl-0 = pinctrl_gem0_default;
   
  ethernet_phy: ethernet-phy@7 {
  reg = 7;
  };
   };
   
  +gpio0 {
  +   pinctrl-names = default;
  +   pinctrl-0 = pinctrl_gpio0_default;
 
 On linux-next the equivalent no longer works for the Parallella, with
 gpio failing to probe.
 
 If I move these two properties to the leds node (for which I am
 configuring gpio 7) then I get a heartbeat as before.

Moving this away from the gpio node seems wrong. IMHO, it should be
where it is. I guess, there might be some issues with probe
ordering/deferral that we may have to sort out.

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

Re: [PATCH v5] can: Convert to runtime_pm

[Sören Brinkmann]

On Mon, 2015-01-12 at 08:34PM +0530, Kedareswara rao Appana wrote:
 Instead of enabling/disabling clocks at several locations in the driver,
 Use the runtime_pm framework. This consolidates the actions for runtime PM
 In the appropriate callbacks and makes the driver more readable and 
 mantainable.
 
 Signed-off-by: Soren Brinkmann soren.brinkm...@xilinx.com
 Signed-off-by: Kedareswara rao Appana appa...@xilinx.com
 ---
 Changes for v5:
  - Updated with the review comments.
Updated the remove fuction to use runtime_pm.
 Chnages for v4:
  - Updated with the review comments.
 Changes for v3:
   - Converted the driver to use runtime_pm.
 Changes for v2:
   - Removed the struct platform_device* from suspend/resume
 as suggest by Lothar.
 
  drivers/net/can/xilinx_can.c |  157 -
  1 files changed, 107 insertions(+), 50 deletions(-)
[..]
 +static int __maybe_unused xcan_runtime_resume(struct device *dev)
  {
 - struct platform_device *pdev = dev_get_drvdata(dev);
 - struct net_device *ndev = platform_get_drvdata(pdev);
 + struct net_device *ndev = dev_get_drvdata(dev);
   struct xcan_priv *priv = netdev_priv(ndev);
   int ret;
 + u32 isr, status;
  
   ret = clk_enable(priv-bus_clk);
   if (ret) {
 @@ -1014,15 +1030,28 @@ static int __maybe_unused xcan_resume(struct device 
 *dev)
   ret = clk_enable(priv-can_clk);
   if (ret) {
   dev_err(dev, Cannot enable clock.\n);
 - clk_disable_unprepare(priv-bus_clk);
 + clk_disable(priv-bus_clk);
[...]
 @@ -1173,12 +1219,23 @@ static int xcan_remove(struct platform_device *pdev)
  {
   struct net_device *ndev = platform_get_drvdata(pdev);
   struct xcan_priv *priv = netdev_priv(ndev);
 + int ret;
 +
 + ret = pm_runtime_get_sync(pdev-dev);
 + if (ret  0) {
 + netdev_err(ndev, %s: pm_runtime_get failed(%d)\n,
 + __func__, ret);
 + return ret;
 + }
  
   if (set_reset_mode(ndev)  0)
   netdev_err(ndev, mode resetting failed!\n);
  
   unregister_candev(ndev);
 + pm_runtime_disable(pdev-dev);
   netif_napi_del(priv-napi);
 + clk_disable_unprepare(priv-bus_clk);
 + clk_disable_unprepare(priv-can_clk);

Shouldn't pretty much all these occurrences of clk_disable/enable
disappear? This should all be handled by the runtime_pm framework now.

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

Re: [PATCH v4 0/7] pinctrl support for Zynq

[Sören Brinkmann]

On Sun, 2015-01-11 at 10:41PM +0100, Linus Walleij wrote:
 On Fri, Jan 9, 2015 at 5:53 PM, Sören Brinkmann
 soren.brinkm...@xilinx.com wrote:
  On Fri, 2015-01-09 at 07:43AM -0800, Soren Brinkmann wrote:
  Hi Linus,
 
  I rebased my branch onto your current pinctrl/devel. Other than that
  this should be exactly what has been sent out as v3. This time I'll use
  the gmail SMTP again, which hopefully resolves the encoding issue.
 
  The transfer-encoding is again 'quoted-printable'. So, seems it is not
  exchange to blame. I tried to git am a patch from this series and it
  seems to work fine. I.e. send-email and am apparently to the right
  thing.
 
 Yeah it works like a charm :)
 
 All patches that affect the pin control tree are applied now. It is
 looking *beautiful* especially your device trees look exactly as I
 want them to look for a group/function type pin controller driver.

Thanks a lot! This is great.

@Michal: Could you please pick up the patches that go through the armsoc
tree (5 and 6 in this series)?

Thanks,
Sören
--
To unsubscribe from this list: send the line unsubscribe devicetree in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 0/7] Pinctrl support for Zynq

[Sören Brinkmann]

On Fri, 2015-01-09 at 09:45AM +0100, Linus Walleij wrote:
 On Fri, Dec 12, 2014 at 5:37 PM, Sören Brinkmann
 soren.brinkm...@xilinx.com wrote:
 
  Argh, this went through the wrong SMTP. I suspect threading is broken
  due to that. Let me know if you want me to resend this with intact
  threading.
 
 I don't care much about the threading, but all messages are quoted-printable
 which doesn't apply :(
 
 i.e.:
 
 Content-Transfer-Encoding: quoted-printable
 (...)
 
 + const char *subnode_target_type =3D pins;
 =20
 (...)
 
 Can you resent with more sane encoding? I don't know where this
 comes from but suspect the Microsoft Exchange SMTP server
 does this when you use diacritics and german letters...

Okay, I'll rebase to the latest pinctrl/devel branch (if needed) and
send an updated version bypassing exchange. I hope that works then.

Thanks,
Sören
--
To unsubscribe from this list: send the line unsubscribe devicetree in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 0/7] pinctrl support for Zynq

[Sören Brinkmann]

On Fri, 2015-01-09 at 07:43AM -0800, Soren Brinkmann wrote:
 Hi Linus,
 
 I rebased my branch onto your current pinctrl/devel. Other than that
 this should be exactly what has been sent out as v3. This time I'll use
 the gmail SMTP again, which hopefully resolves the encoding issue.

The transfer-encoding is again 'quoted-printable'. So, seems it is not
exchange to blame. I tried to git am a patch from this series and it
seems to work fine. I.e. send-email and am apparently to the right
thing.
Let me know if that doesn't work for you. In that case I might have to
get rid of all non-ascii characters (quite sad, given that it's already
2015 and things like this still don't work right away).

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

Re: [PATCH v3 0/7] Pinctrl support for Zynq

[Sören Brinkmann]

Argh, this went through the wrong SMTP. I suspect threading is broken
due to that. Let me know if you want me to resend this with intact
threading.
My apologies.

Thanks,
Sören

On Fri, 2014-12-12 at 08:34AM -0800, Soren Brinkmann wrote:
 Hi,
 
 I made those few adjustments that came up in the v2 review. Other than
 those, these are the same patches as before.
 The series is based on pinctrl-v3.19-1, the current pinctrl/devel
 branch.
 A branch with these changes is available at
 https://github.com/sorenb-xlnx/linux-xlnx/commits/pinctrl
 
   Thanks,
   Sören
 
 Soren Brinkmann (7):
   pinctrl: pinconf-generic: Infer map type from DT property
   pinctrl: pinconf-generic: Allow driver to specify DT params
   pinctrl: zynq: Document DT binding
   pinctrl: Add driver for Zynq
   ARM: zynq: Enable pinctrl
   ARM: zynq: DT: Add pinctrl information
   pinctrl: qcom-spmi-gpio: Migrate to pinconf-generic
 
  .../bindings/pinctrl/xlnx,zynq-pinctrl.txt |  104 ++
  arch/arm/boot/dts/zynq-7000.dtsi   |8 +-
  arch/arm/boot/dts/zynq-zc702.dts   |  181 +++
  arch/arm/boot/dts/zynq-zc706.dts   |  152 +++
  arch/arm/mach-zynq/Kconfig |2 +
  drivers/pinctrl/Kconfig|8 +
  drivers/pinctrl/Makefile   |1 +
  drivers/pinctrl/nomadik/pinctrl-abx500.c   |2 +-
  drivers/pinctrl/pinconf-generic.c  |  199 ++--
  drivers/pinctrl/pinconf.c  |4 +-
  drivers/pinctrl/pinconf.h  |   22 +-
  drivers/pinctrl/pinctrl-rockchip.c |2 +-
  drivers/pinctrl/pinctrl-tz1090-pdc.c   |2 +-
  drivers/pinctrl/pinctrl-tz1090.c   |2 +-
  drivers/pinctrl/pinctrl-zynq.c | 1176 
 
  drivers/pinctrl/qcom/pinctrl-spmi-gpio.c   |  125 +--
  drivers/pinctrl/sh-pfc/pinctrl.c   |2 +-
  include/linux/pinctrl/pinconf-generic.h|   29 +
  include/linux/pinctrl/pinctrl.h|9 +
  19 files changed, 1808 insertions(+), 222 deletions(-)
  create mode 100644 
 Documentation/devicetree/bindings/pinctrl/xlnx,zynq-pinctrl.txt
  create mode 100644 drivers/pinctrl/pinctrl-zynq.c
 
 -- 
 2.2.0.1.g9ee0458
 
 --
 To unsubscribe from this list: send the line unsubscribe devicetree in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe devicetree in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM: zynq: DT: Add USB to device tree

[Sören Brinkmann]

On Mon, 2014-12-01 at 09:26PM +0100, Andreas Färber wrote:
 Hi Sören,
 
 Am 01.12.2014 um 19:42 schrieb Soren Brinkmann:
  Add USB nodes to zc702, zc706 and zed device trees.
  
  Signed-off-by: Soren Brinkmann soren.brinkm...@xilinx.com
  ---
   arch/arm/boot/dts/zynq-7000.dtsi | 22 +-
   arch/arm/boot/dts/zynq-zc702.dts | 11 +++
   arch/arm/boot/dts/zynq-zc706.dts | 10 ++
   arch/arm/boot/dts/zynq-zed.dts   | 10 ++
   4 files changed, 52 insertions(+), 1 deletion(-)
  
  diff --git a/arch/arm/boot/dts/zynq-7000.dtsi 
  b/arch/arm/boot/dts/zynq-7000.dtsi
  index ce2ef5bec4f2..9ed60938f301 100644
  --- a/arch/arm/boot/dts/zynq-7000.dtsi
  +++ b/arch/arm/boot/dts/zynq-7000.dtsi
  @@ -315,6 +315,26 @@
  clocks = clkc 4;
  };
   
  +   usb0: usb@e0002000 {
  +   compatible = xlnx,zynq-usb-2.20a, chipidea,usb2;
  +   status = disabled;
  +   clocks = clkc 28;
  +   interrupt-parent = intc;
  +   interrupts = 0 21 4;
  +   reg = 0xe0002000 0x1000;
  +   phy_type = ulpi;
  +   };
  +
  +   usb1: usb@e0003000 {
  +   compatible = xlnx,zynq-usb-2.20a, chipidea,usb2;
  +   status = disabled;
  +   clocks = clkc 29;
  +   interrupt-parent = intc;
  +   interrupts = 0 44 4;
  +   reg = 0xe0003000 0x1000;
  +   phy_type = ulpi;
  +   };
  +
  watchdog0: watchdog@f8005000 {
  clocks = clkc 45;
  compatible = xlnx,zynq-wdt-r1p2;
 
 This part looks good.
 
  @@ -324,6 +344,6 @@
  reg = 0xf8005000 0x1000;
  reset = 0;
  timeout-sec = 10;
  -   };
  +   } ;
  };
   };
 
 Unrelated accidental change here though. :)
 
 Did I miss a matching series actually implementing the driver? I'd need
 to test the latest version to determine how to update the Parallella
 device tree - I assume that USB0 is in dr_mode=host, but with the
 previous patchsets I was unable to verify. CC'ing Ola.

It was merged to the USB tree recently. I tested this on linux-next.

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

Re: [PATCH] ARM: zynq: DT: Add USB to device tree

[Sören Brinkmann]

Hi Arnd,

On Mon, 2014-12-01 at 10:26PM +0100, Arnd Bergmann wrote:
 On Monday 01 December 2014 10:42:32 Soren Brinkmann wrote:
  +   usb_phy0: usb-phy@0 {
  +   compatible = usb-nop-xceiv;
  +   #phy-cells = 0;
  +   };
   };
 
 As discussed in an unrelated thread today, please drop the @0 in the
 node name, since the device has no 'reg' property.

What is the best practice for naming such nodes then? On these boards
it's not the case, but Zynq has two USB cores. So, there may be DTs that
will have two phys in there. Would we just do 'usb-phy-0'?

Thanks,
Sören
--
To unsubscribe from this list: send the line unsubscribe devicetree in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM: zynq: DT: Add USB to device tree

[Sören Brinkmann]

On Mon, 2014-12-01 at 01:24PM -0800, Sören Brinkmann wrote:
 On Mon, 2014-12-01 at 09:26PM +0100, Andreas Färber wrote:
  Hi Sören,
  
  Am 01.12.2014 um 19:42 schrieb Soren Brinkmann:
   Add USB nodes to zc702, zc706 and zed device trees.
   
   Signed-off-by: Soren Brinkmann soren.brinkm...@xilinx.com
   ---
arch/arm/boot/dts/zynq-7000.dtsi | 22 +-
arch/arm/boot/dts/zynq-zc702.dts | 11 +++
arch/arm/boot/dts/zynq-zc706.dts | 10 ++
arch/arm/boot/dts/zynq-zed.dts   | 10 ++
4 files changed, 52 insertions(+), 1 deletion(-)
   
   diff --git a/arch/arm/boot/dts/zynq-7000.dtsi 
   b/arch/arm/boot/dts/zynq-7000.dtsi
   index ce2ef5bec4f2..9ed60938f301 100644
   --- a/arch/arm/boot/dts/zynq-7000.dtsi
   +++ b/arch/arm/boot/dts/zynq-7000.dtsi
[...]
   @@ -324,6 +344,6 @@
 reg = 0xf8005000 0x1000;
 reset = 0;
 timeout-sec = 10;
   - };
   + } ;
 };
};
  
  Unrelated accidental change here though. :)

Since a v2 is needed now, does this really require a dedicated patch to
fix this bogus space here or do we just look the other way?

Thanks,
Sören
--
To unsubscribe from this list: send the line unsubscribe devicetree in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM: zynq: DT: Add USB to device tree

[Sören Brinkmann]

On Tue, 2014-12-02 at 12:14AM +0100, Andreas Färber wrote:
 Am 01.12.2014 um 23:56 schrieb Sören Brinkmann:
  On Mon, 2014-12-01 at 01:24PM -0800, Sören Brinkmann wrote:
  On Mon, 2014-12-01 at 09:26PM +0100, Andreas Färber wrote:
  Am 01.12.2014 um 19:42 schrieb Soren Brinkmann:
  Add USB nodes to zc702, zc706 and zed device trees.
 
  Signed-off-by: Soren Brinkmann soren.brinkm...@xilinx.com
  ---
   arch/arm/boot/dts/zynq-7000.dtsi | 22 +-
   arch/arm/boot/dts/zynq-zc702.dts | 11 +++
   arch/arm/boot/dts/zynq-zc706.dts | 10 ++
   arch/arm/boot/dts/zynq-zed.dts   | 10 ++
   4 files changed, 52 insertions(+), 1 deletion(-)
 
  diff --git a/arch/arm/boot/dts/zynq-7000.dtsi 
  b/arch/arm/boot/dts/zynq-7000.dtsi
  index ce2ef5bec4f2..9ed60938f301 100644
  --- a/arch/arm/boot/dts/zynq-7000.dtsi
  +++ b/arch/arm/boot/dts/zynq-7000.dtsi
  [...]
  @@ -324,6 +344,6 @@
   reg = 0xf8005000 0x1000;
   reset = 0;
   timeout-sec = 10;
  -};
  +} ;
   };
   };
 
  Unrelated accidental change here though. :)
  
  Since a v2 is needed now, does this really require a dedicated patch to
  fix this bogus space here or do we just look the other way?
 
 Sorry, I don't understand the question: If as you say a v2 is needed,
 why knowingly introduce a space between } and ; in your patch? Maybe I'm
 missing something or you're looking at the patch in reverse...?

Oh, I thought I was removing it... That must have been some merge error.
Forget what I said. This part will vanish.

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

Re: [PATCH v3] can: Convert to runtime_pm

[Sören Brinkmann]

Hi Kedar,

On Thu, 2014-11-27 at 06:38PM +0530, Kedareswara rao Appana wrote:
 Instead of enabling/disabling clocks at several locations in the driver,
 use the runtime_pm framework. This consolidates the actions for
 runtime PM in the appropriate callbacks and makes the driver more
 readable and mantainable.
 
 Signed-off-by: Soren Brinkmann soren.brinkm...@xilinx.com
 Signed-off-by: Kedareswara rao Appana appa...@xilinx.com
 ---
 Changes for v3:
   - Converted the driver to use runtime_pm.
 Changes for v2:
   - Removed the struct platform_device* from suspend/resume
 as suggest by Lothar.
 
  drivers/net/can/xilinx_can.c |  119 +
  1 files changed, 72 insertions(+), 47 deletions(-)
 
 diff --git a/drivers/net/can/xilinx_can.c b/drivers/net/can/xilinx_can.c
 index 8a998e3..1be28ed 100644
 --- a/drivers/net/can/xilinx_can.c
 +++ b/drivers/net/can/xilinx_can.c
 @@ -32,6 +32,7 @@
  #include linux/can/dev.h
  #include linux/can/error.h
  #include linux/can/led.h
 +#include linux/pm_runtime.h
  
  #define DRIVER_NAME  xilinx_can
  
 @@ -138,7 +139,7 @@ struct xcan_priv {
   u32 (*read_reg)(const struct xcan_priv *priv, enum xcan_reg reg);
   void (*write_reg)(const struct xcan_priv *priv, enum xcan_reg reg,
   u32 val);
 - struct net_device *dev;
 + struct device *dev;
   void __iomem *reg_base;
   unsigned long irq_flags;
   struct clk *bus_clk;
 @@ -842,6 +843,13 @@ static int xcan_open(struct net_device *ndev)
   struct xcan_priv *priv = netdev_priv(ndev);
   int ret;
  
 + ret = pm_runtime_get_sync(priv-dev);
 + if (ret  0) {
 + netdev_err(ndev, %s: runtime CAN resume failed(%d)\n\r,

There might be other issues than the resume that make this fail. It
should probably just say 'pm_runtime_get failed'.
The CAN in the string should not be needed, the netdev_err macro makes
sure the device name is printed.
Can we have a space between 'failed' and the error code?
There should not be a '\r'

 + __func__, ret);
 + return ret;
 + }
 +
   ret = request_irq(ndev-irq, xcan_interrupt, priv-irq_flags,
   ndev-name, ndev);
   if (ret  0) {
 @@ -849,29 +857,17 @@ static int xcan_open(struct net_device *ndev)
   goto err;
   }
  
 - ret = clk_prepare_enable(priv-can_clk);
 - if (ret) {
 - netdev_err(ndev, unable to enable device clock\n);
 - goto err_irq;
 - }
 -
 - ret = clk_prepare_enable(priv-bus_clk);
 - if (ret) {
 - netdev_err(ndev, unable to enable bus clock\n);
 - goto err_can_clk;
 - }
 -
   /* Set chip into reset mode */
   ret = set_reset_mode(ndev);
   if (ret  0) {
   netdev_err(ndev, mode resetting failed!\n);
 - goto err_bus_clk;
 + goto err_irq;
   }
  
   /* Common open */
   ret = open_candev(ndev);
   if (ret)
 - goto err_bus_clk;
 + goto err_irq;
  
   ret = xcan_chip_start(ndev);
   if (ret  0) {
 @@ -887,13 +883,11 @@ static int xcan_open(struct net_device *ndev)
  
  err_candev:
   close_candev(ndev);
 -err_bus_clk:
 - clk_disable_unprepare(priv-bus_clk);
 -err_can_clk:
 - clk_disable_unprepare(priv-can_clk);
  err_irq:
   free_irq(ndev-irq, ndev);
  err:
 + pm_runtime_put(priv-dev);
 +
   return ret;
  }
  
 @@ -910,12 +904,11 @@ static int xcan_close(struct net_device *ndev)
   netif_stop_queue(ndev);
   napi_disable(priv-napi);
   xcan_chip_stop(ndev);
 - clk_disable_unprepare(priv-bus_clk);
 - clk_disable_unprepare(priv-can_clk);
   free_irq(ndev-irq, ndev);
   close_candev(ndev);
  
   can_led_event(ndev, CAN_LED_EVENT_STOP);
 + pm_runtime_put(priv-dev);
  
   return 0;
  }
 @@ -934,27 +927,21 @@ static int xcan_get_berr_counter(const struct 
 net_device *ndev,
   struct xcan_priv *priv = netdev_priv(ndev);
   int ret;
  
 - ret = clk_prepare_enable(priv-can_clk);
 - if (ret)
 - goto err;
 + ret = pm_runtime_get_sync(priv-dev);
 + if (ret  0) {
 + netdev_err(ndev, %s: runtime resume failed(%d)\n\r,
 + __func__, ret);

As above.

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

Re: [PATCH v3] can: Convert to runtime_pm

[Sören Brinkmann]

On Thu, 2014-11-27 at 10:17PM +0100, Marc Kleine-Budde wrote:
 On 11/27/2014 02:08 PM, Kedareswara rao Appana wrote:
  Instead of enabling/disabling clocks at several locations in the driver,
  use the runtime_pm framework. This consolidates the actions for
  runtime PM in the appropriate callbacks and makes the driver more
  readable and mantainable.
  
  Signed-off-by: Soren Brinkmann soren.brinkm...@xilinx.com
  Signed-off-by: Kedareswara rao Appana appa...@xilinx.com
  ---
  Changes for v3:
- Converted the driver to use runtime_pm.
  Changes for v2:
- Removed the struct platform_device* from suspend/resume
  as suggest by Lothar.
  
   drivers/net/can/xilinx_can.c |  119 
  +
   1 files changed, 72 insertions(+), 47 deletions(-)
  
  diff --git a/drivers/net/can/xilinx_can.c b/drivers/net/can/xilinx_can.c
  index 8a998e3..1be28ed 100644
  --- a/drivers/net/can/xilinx_can.c
  +++ b/drivers/net/can/xilinx_can.c
[...]
  @@ -1030,7 +1046,10 @@ static int __maybe_unused xcan_resume(struct device 
  *dev)
  return 0;
   }
   
  -static SIMPLE_DEV_PM_OPS(xcan_dev_pm_ops, xcan_suspend, xcan_resume);
  +static const struct dev_pm_ops xcan_dev_pm_ops = {
  +   SET_SYSTEM_SLEEP_PM_OPS(xcan_suspend, xcan_resume)
  +   SET_PM_RUNTIME_PM_OPS(xcan_runtime_suspend, xcan_runtime_resume, NULL)
  +};
   
   /**
* xcan_probe - Platform registration call
  @@ -1071,7 +1090,7 @@ static int xcan_probe(struct platform_device *pdev)
  return -ENOMEM;
   
  priv = netdev_priv(ndev);
  -   priv-dev = ndev;
  +   priv-dev = pdev-dev;
  priv-can.bittiming_const = xcan_bittiming_const;
  priv-can.do_set_mode = xcan_do_set_mode;
  priv-can.do_get_berr_counter = xcan_get_berr_counter;
  @@ -1137,6 +1156,11 @@ static int xcan_probe(struct platform_device *pdev)
   
  netif_napi_add(ndev, priv-napi, xcan_rx_poll, rx_max);
   
  +   pm_runtime_set_active(pdev-dev);
  +   pm_runtime_irq_safe(pdev-dev);
 
 You use just clock_enable()/disable() in the runtime functions, thus you
 can say they are irq_safe. On the other the the zync grpio driver uses
 full prepare_enable/disable_unprepare calls. What's best practice here?

IIRC, the prepare/unprepare functions can sleep. xcan_get_berr_counter
is called from atomic context. So, I think we have to use the
disable/enable functions without the prepare/unprepare.
In the GPIO driver the that problem does not exist.

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

Re: [PATCH v3] can: Convert to runtime_pm

[Sören Brinkmann]

On Thu, 2014-11-27 at 11:55PM +0100, Marc Kleine-Budde wrote:
 On 11/27/2014 11:47 PM, Sören Brinkmann wrote:
  On Thu, 2014-11-27 at 10:17PM +0100, Marc Kleine-Budde wrote:
  On 11/27/2014 02:08 PM, Kedareswara rao Appana wrote:
  Instead of enabling/disabling clocks at several locations in the driver,
  use the runtime_pm framework. This consolidates the actions for
  runtime PM in the appropriate callbacks and makes the driver more
  readable and mantainable.
 
  Signed-off-by: Soren Brinkmann soren.brinkm...@xilinx.com
  Signed-off-by: Kedareswara rao Appana appa...@xilinx.com
  ---
  Changes for v3:
- Converted the driver to use runtime_pm.
  Changes for v2:
- Removed the struct platform_device* from suspend/resume
  as suggest by Lothar.
 
   drivers/net/can/xilinx_can.c |  119 
  +
   1 files changed, 72 insertions(+), 47 deletions(-)
 
  diff --git a/drivers/net/can/xilinx_can.c b/drivers/net/can/xilinx_can.c
  index 8a998e3..1be28ed 100644
  --- a/drivers/net/can/xilinx_can.c
  +++ b/drivers/net/can/xilinx_can.c
  [...]
  @@ -1030,7 +1046,10 @@ static int __maybe_unused xcan_resume(struct 
  device *dev)
return 0;
   }
   
  -static SIMPLE_DEV_PM_OPS(xcan_dev_pm_ops, xcan_suspend, xcan_resume);
  +static const struct dev_pm_ops xcan_dev_pm_ops = {
  + SET_SYSTEM_SLEEP_PM_OPS(xcan_suspend, xcan_resume)
  + SET_PM_RUNTIME_PM_OPS(xcan_runtime_suspend, xcan_runtime_resume, NULL)
  +};
   
   /**
* xcan_probe - Platform registration call
  @@ -1071,7 +1090,7 @@ static int xcan_probe(struct platform_device *pdev)
return -ENOMEM;
   
priv = netdev_priv(ndev);
  - priv-dev = ndev;
  + priv-dev = pdev-dev;
priv-can.bittiming_const = xcan_bittiming_const;
priv-can.do_set_mode = xcan_do_set_mode;
priv-can.do_get_berr_counter = xcan_get_berr_counter;
  @@ -1137,6 +1156,11 @@ static int xcan_probe(struct platform_device *pdev)
   
netif_napi_add(ndev, priv-napi, xcan_rx_poll, rx_max);
   
  + pm_runtime_set_active(pdev-dev);
  + pm_runtime_irq_safe(pdev-dev);
 
  You use just clock_enable()/disable() in the runtime functions, thus you
  can say they are irq_safe. On the other the the zync grpio driver uses
  full prepare_enable/disable_unprepare calls. What's best practice here?
  
  IIRC, the prepare/unprepare functions can sleep. xcan_get_berr_counter
  is called from atomic context. So, I think we have to use the
  disable/enable functions without the prepare/unprepare.
  In the GPIO driver the that problem does not exist.
 
 IC, yes, correct.
 
 This is why we introducted in other drivers a __get_berr_counter()
 function, that doesn't touch the clocks, which is used from within the
 driver (from the atomic contects), while get_berr_counter() will fiddle
 with the clocks. This function is used for the
 priv-can.do_get_berr_counter callback.

I have the feeling I'm missing something. If I remove the 'must not
sleep' requirement from the runtime suspend/resume functions, I get
this:

BUG: sleeping function called from invalid context at 
drivers/base/power/runtime.c:954
in_atomic(): 0, irqs_disabled(): 0, pid: 161, name: ip
INFO: lockdep is turned off.
CPU: 0 PID: 161 Comm: ip Not tainted 
3.18.0-rc1-xilinx-00059-g21da26693b61-dirty #104
[c00186a8] (unwind_backtrace) from [c00139f4] (show_stack+0x20/0x24)
[c00139f4] (show_stack) from [c055a41c] (dump_stack+0x8c/0xd0)
[c055a41c] (dump_stack) from [c0054808] (__might_sleep+0x1ac/0x1e4)
[c0054808] (__might_sleep) from [c034f8f0] (__pm_runtime_resume+0x40/0x9c)
[c034f8f0] (__pm_runtime_resume) from [c03b48d8] 
(xcan_get_berr_counter+0x2c/0x9c)
[c03b48d8] (xcan_get_berr_counter) from [c03b2ecc] 
(can_fill_info+0x160/0x1f4)
[c03b2ecc] (can_fill_info) from [c049f3b0] (rtnl_fill_ifinfo+0x794/0x970)
[c049f3b0] (rtnl_fill_ifinfo) from [c04a0048] (rtnl_dump_ifinfo+0x1b4/0x2fc)
[c04a0048] (rtnl_dump_ifinfo) from [c04af9c8] (netlink_dump+0xe4/0x270)
[c04af9c8] (netlink_dump) from [c04b0764] (__netlink_dump_start+0xdc/0x170)
[c04b0764] (__netlink_dump_start) from [c04a1fc4] 
(rtnetlink_rcv_msg+0x154/0x1e0)
[c04a1fc4] (rtnetlink_rcv_msg) from [c04b1e88] (netlink_rcv_skb+0x68/0xc4)
[c04b1e88] (netlink_rcv_skb) from [c04a045c] (rtnetlink_rcv+0x28/0x34)
[c04a045c] (rtnetlink_rcv) from [c04b1770] (netlink_unicast+0x144/0x210)
[c04b1770] (netlink_unicast) from [c04b1c9c] (netlink_sendmsg+0x394/0x414)
[c04b1c9c] (netlink_sendmsg) from [c046ffcc] (sock_sendmsg+0x8c/0xc0)
[c046ffcc] (sock_sendmsg) from [c04726bc] (SyS_sendto+0xd8/0x114)
[c04726bc] (SyS_sendto) from [c000f3e0] (ret_fast_syscall+0x0/0x48)

I.e. the core calls this function from atomic context. And in an earlier
thread you said the core can also call this before/after calling the
open/close callbacks (which applies here too, I think).

I think the callback is required to
 - not sleep
 - get the device in a power state that allows querying its registers

So, I don't see how splitting the xcan_get_berr_counter callback helps

Re: [PATCH v3 0/3] Amlogic Meson pinctrl driver

[Sören Brinkmann]

Hi Beniamino,

On Sun, 2014-11-16 at 09:14PM +0100, Beniamino Galvani wrote:
 Hi,
 
 this is the third version of the pinctrl driver for Amlogic Meson. It
 provides a common infrastructure than can be used by all the SoCs of
 the family and configuration data specific for Meson8. Adding support
 for other SoCs should be only matter of defining new pins and banks.
 
 The driver uses generic DT properties for the definition of mux and
 configuration nodes; since there is an ongoing effort to centralize
 the parsing to generic code, I've reused the patch pinctrl:
 pinconf-generic: Infer map type from DT property [1] posted a few
 days ago by Soren Brinkmann. I will update the series if that patch is
 going to change after the mailing list discussion.

Looks really nice to me and it's great that you use my patch as basis.
Looks like it works for more people than just me :)
I hope we can convince Linus to take that patch more or less as is. Even
though kludgy I don't see an easy way for addressing Linus' comments
without breaking current users of the interface. It may be easier/better
to gradually migrate to the newly defined bindings and remove the
kludgyness as user that rely on the current behavior disappear.

Thanks,
Sören
--
To unsubscribe from this list: send the line unsubscribe devicetree in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 6/6] ARM: zynq: DT: Add OCM controller node

[Sören Brinkmann]

On Sun, 2014-11-16 at 11:51AM +0100, Andreas Färber wrote:
 Hi Michal,
 
 Am 14.11.2014 um 11:52 schrieb Michal Simek:
  diff --git a/arch/arm/boot/dts/zynq-7000.dtsi 
  b/arch/arm/boot/dts/zynq-7000.dtsi
  index ce2ef5bec4f2..e217fb1c1169 100644
  --- a/arch/arm/boot/dts/zynq-7000.dtsi
  +++ b/arch/arm/boot/dts/zynq-7000.dtsi
  @@ -150,6 +150,13 @@
  reg = 0xf8006000 0x1000;
  };
  
  +   ocmc: memory-controller@f800c000 {
  +   compatible = xlnx,zynq-ocmc-1.0;
  +   interrupt-parent = intc;
  +   interrupts = 0 3 4;
  +   reg = 0xf800c000 0x1000;
  +   };
  +
  uart0: serial@e000 {
  compatible = xlnx,xuartps, cdns,uart-r1p8;
  status = disabled;
 
 Not directly related to this patch: As one can see here, the node order
 is quite a mess... According to Olof, nodes should be ordered by unit
 address, whereas here some but not all seem ordered by node name. Would
 you welcome a cleanup patch, or can you fix that yourself?

I wouldn't say it's a mess, just a different property to sort the nodes
by. For humans reading the DT, searching for nodes, alphabetical order
helps finding the right node, IMHO. What advantage would sorting by
address have?

Thanks,
Sören
--
To unsubscribe from this list: send the line unsubscribe devicetree in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] can: Fix bug in suspend/resume

[Sören Brinkmann]

On Fri, 2014-11-14 at 09:54AM +0100, Marc Kleine-Budde wrote:
 On 11/14/2014 09:16 AM, Kedareswara rao Appana wrote:
  The drvdata in the suspend/resume is of type struct net_device,
  not the platform device.Enable the clocks in the suspend before
  accessing the registers of the CAN.
  
  Signed-off-by: Kedareswara rao Appana appa...@xilinx.com
  ---
  Changes for v2:
- Removed the struct platform_device* from suspend/resume
  as suggest by Lothar.
- The clocks are getting disabled and un prepared at the end of the 
  probe. 
  In the suspend the driver is doing a register write.In order
  To do that register write we have to again enable and prepare the 
  clocks.
 
 Please look the at suspend/resume code and count the
 clock_enable/disable manually. After a suspend/resume cycle, you have
 enabled the clock twice, but disabled it once.
 
 I think you have to abstract the clock handling behind runtime PM. I
 haven't done this myself yet, but the strong feeling that this is a
 possible solution to your problem. These links might help:

I agree, the clock handling looks weird. Also the clk_disable calls in
xcan_get_berr_counter() look suspicious to me, but I might be wrong.
I think you can take a look at gpio-zynq for an example for runtime_pm
usage. I think the usage model in that driver is similar to here.

Thanks,
Sören
--
To unsubscribe from this list: send the line unsubscribe devicetree in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] can: Fix bug in suspend/resume

[Sören Brinkmann]

On Fri, 2014-11-14 at 04:09PM +0100, Marc Kleine-Budde wrote:
 On 11/14/2014 04:05 PM, Sören Brinkmann wrote:
  On Fri, 2014-11-14 at 09:54AM +0100, Marc Kleine-Budde wrote:
  On 11/14/2014 09:16 AM, Kedareswara rao Appana wrote:
  The drvdata in the suspend/resume is of type struct net_device,
  not the platform device.Enable the clocks in the suspend before
  accessing the registers of the CAN.
 
  Signed-off-by: Kedareswara rao Appana appa...@xilinx.com
  ---
  Changes for v2:
- Removed the struct platform_device* from suspend/resume
  as suggest by Lothar.
- The clocks are getting disabled and un prepared at the end of the 
  probe. 
  In the suspend the driver is doing a register write.In order
  To do that register write we have to again enable and prepare the 
  clocks.
 
  Please look the at suspend/resume code and count the
  clock_enable/disable manually. After a suspend/resume cycle, you have
  enabled the clock twice, but disabled it once.
 
  I think you have to abstract the clock handling behind runtime PM. I
  haven't done this myself yet, but the strong feeling that this is a
  possible solution to your problem. These links might help:
  
  I agree, the clock handling looks weird. Also the clk_disable calls in
  xcan_get_berr_counter() look suspicious to me, but I might be wrong.
  I think you can take a look at gpio-zynq for an example for runtime_pm
  usage. I think the usage model in that driver is similar to here.
 
 The xcan_get_berr_counter() function is correct, when doing manual (i.e.
 non runtime-pm) clock handling. This function might be called if the
 interface is down, this means clocks are disabled.

I see, thanks for the clarification. Guess that should become
pm_runtime_get_sync() and pm_runtime_put() when converting to
runtime_pm.

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

Re: [PATCH RESEND] Documentation: devicetree: Fix Xilinx VDMA specification

[Sören Brinkmann]

On Wed, 2014-11-12 at 02:51PM +0100, Andreas Färber wrote:
 The specification requires xlnx,data-width, but example and driver use
 xlnx,datawidth. Change the specification to match the implementation.

Isn't this the wrong way around? The bindings are considered API, so
shouldn't the driver be fixed to match the spec?
Are there already dts files out there using either of these options?

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

Re: [PATCH RESEND] Documentation: devicetree: Fix Xilinx VDMA specification

[Sören Brinkmann]

On Wed, 2014-11-12 at 07:03PM +0100, Andreas Färber wrote:
 Am 12.11.2014 um 16:57 schrieb Sören Brinkmann:
  On Wed, 2014-11-12 at 02:51PM +0100, Andreas Färber wrote:
  The specification requires xlnx,data-width, but example and driver use
  xlnx,datawidth. Change the specification to match the implementation.
  
  Isn't this the wrong way around? The bindings are considered API, so
  shouldn't the driver be fixed to match the spec?
 
 In theory, patch review should've never let the two differ... ;)
 
 It's not my driver, so I fixed the perceived inconsistency the least
 invasive way; Michal and Srikanth seemed to concur at the time.
 
 https://patchwork.kernel.org/patch/4620261/
 
  Are there already dts files out there using either of these options?
 
 In upstream, no. microblaze and virtex440 use a
 xlnx,include-datawidth-matching-0 property as precedence for the
 spelling, whereas there is an fsl,data-width and an unused msix-data-width.
 
 Downstream, yes: Beyond my own patch derived from the Parallella tree,
 there's some in the ADI tree. None in the Xilinx tree on quick check.
 
 I haven't encountered any using the documented xlnx,data-width - but
 this patch was authored pre 3.17, haven't ran a full Web search again.

grepping through linux-next shows some usage of xlnx,datawidth, but
only the single hit in  Documentation for xlnx,data-width.
Other than VDMA the other hit seems to be just some DT
documentation which I can't find a driver for...
Anyhow, this patch is probably the best way of fixing this.

Reviewed-by: Soren Brinkmann soren.brinkm...@xilinx.com

Thanks,
Sören
--
To unsubscribe from this list: send the line unsubscribe devicetree in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM: dts: zynq: Enable PL clocks for Parallella

[Sören Brinkmann]

Hi Andreas,

On Thu, 2014-11-06 at 06:22PM +0100, Andreas Färber wrote:
 The Parallella board comes with a U-Boot bootloader that loads one of
 two predefined FPGA bitstreams before booting the kernel. Both define an
 AXI interface to the on-board Epiphany processor.
 
 Enable clocks FCLK0..FCLK3 for the Programmable Logic by default.
 
 Otherwise accessing, e.g., the ESYSRESET register freezes the board,
 as seen with the Epiphany SDK tools e-reset and e-hw-rev, using /dev/mem.

Yeah, this is the problem. Bypassing the kernel banging on memory
directly through /dev/mem does not leave any chance of enabling clocks on
demand through the proper interfaces.

Though, this is a valid workaround for the immediate problem, longer
term, you should consider adding some kind of proper kernel driver for
this interface that would then use the clock framework to control the
required clocks dynamically.

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

Re: [PATCH v2 00/11] ARM: dts: zynq: Prepare Parallella

[Sören Brinkmann]

On Tue, 2014-10-21 at 10:52PM +0200, Andreas Färber wrote:
 Hi Olof and Sören,
 
 Am 18.10.2014 um 06:28 schrieb Olof Johansson:
  On Thu, Jul 24, 2014 at 4:00 PM, Andreas Färber afaer...@suse.de wrote:
  Hello,
 
  This patch series adds an initial device tree for the Parallella board.
  UART, SD card, Ethernet are enabled.
  Not yet enabled are HDMI, QSPI flash and 2x USB.
  
  Andreas (Olofsson) kindly sent me a board, and I added it to the boot
  farm today, it'll be included in boot reports from here on.
 
 Good to hear.
 
  I did a test run with yesterday's -next It looks like networking isn't
  working there at the moment, clock related. Same happens with 3.17 and
  latest mainline, config multi_v7_defconfig:
  
  [WARN] [7.943648] macb e000b000.ethernet eth0: unable to generate
  target frequency: 12500 Hz
  [WARN] [   10.948681] macb e000b000.ethernet eth0: unable to generate
  target frequency: 12500 Hz
 
 I am unable to reproduce this in my setup - at Linus' tip
 c2661b806092d8ea2dccb7b02b65776555e0ee47 (v3.18-rc1-68-gc2661b8), make
 oldconfig'ing my /proc/config.gz.
 
 Sören, in light of your explanations, should I be expecting problems
 with 1GbE or with 100MbE? My board is connected to a 1GbE switch.

Not in general. As I said, I haven't seen that on our boards yet and I
did a little bit of testing making sure that the fundamental code to
switch between frequencies works. I.e. on our zc706 and zc702 boards I
usually can use ethtool to force a certain speed on the interface and
Zynq adjusts to it just fine.

But as I said, it does rely on the divider being sourced with a
frequency that allows generating the required Ethernet clock by
adjusting the 6-bit divider only. If this requirement is not met,
I expect you may see issues like Olof posted.

Are you (Andreas/Olof) using the same bootloaders?

I hope that helps.

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

Re: [PATCH v3 1/2] ARM: zynq: Add OCM controller driver

[Sören Brinkmann]

On Mon, 2014-10-06 at 02:38PM +0200, Michal Simek wrote:
 The driver provide memory allocator which can
 be used by others drivers to allocate memory inside OCM.
 All location for 64kB blocks are supported
 and driver is trying to allocate the largest continuous
 block of memory.
 
 Checking mpcore addressing filterring is not done here
 but could be added in future.
 
 Signed-off-by: Michal Simek michal.si...@xilinx.com
 ---
[...]
 diff --git a/Documentation/devicetree/bindings/arm/zynq/xlnx,zynq-ocmc.txt 
 b/Documentation/devicetree/bindings/arm/zynq/xlnx,zynq-ocmc.txt
 new file mode 100644
 index ..8ddbd1e5ffc1
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/arm/zynq/xlnx,zynq-ocmc.txt
 @@ -0,0 +1,17 @@
 +Device tree bindings for Zynq's OCM controller
 +
 +The OCM is divided to 4 64kB segments which can be separately configured
 +to low or high location. Location is controlled via SLCR.
 +
 +Required properties:
 + compatible: Compatibility string. Must be xlnx,zynq-ocmc-1.0.
 + reg: Specify the base and size of the OCMC registers in the memory map.
 +  E.g.: reg = 0xf800c000 0x1000;
 +
 +Example:
 +ocmc: ocmc@f800c000 {

memory-controller@...

 + compatible =  xlnx,zynq-ocmc-1.0;
 + interrupt-parent = intc;
 + interrupts = 0 3 4;
 + reg = 0xf800c000 0x1000;
 +} ;

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

Re: [PATCH 1/2] ARM: zynq: DT: Add missing reference for ADC

[Sören Brinkmann]

On Wed, 2014-09-24 at 04:01PM +0200, Michal Simek wrote:
 Add missing reference for ADC node.
 
 Signed-off-by: Michal Simek michal.si...@xilinx.com
 ---
 
  arch/arm/boot/dts/zynq-7000.dtsi | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/arch/arm/boot/dts/zynq-7000.dtsi 
 b/arch/arm/boot/dts/zynq-7000.dtsi
 index 772381fe07bb..fc90f47f9c03 100644
 --- a/arch/arm/boot/dts/zynq-7000.dtsi
 +++ b/arch/arm/boot/dts/zynq-7000.dtsi
 @@ -64,7 +64,7 @@
   interrupt-parent = intc;
   ranges;
 
 - adc@f8007100 {
 + adc: adc@f8007100 {
I think we enumerated all labels. I.e. 'adc0' is probably better.

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

Re: [PATCH 2/2] ARM: zynq: DT: Add missing reference for memory-controller

[Sören Brinkmann]

On Wed, 2014-09-24 at 04:01PM +0200, Michal Simek wrote:
 Add missing reference for memory-controller.
 
 Signed-off-by: Michal Simek michal.si...@xilinx.com
 ---
 
  arch/arm/boot/dts/zynq-7000.dtsi | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/arch/arm/boot/dts/zynq-7000.dtsi 
 b/arch/arm/boot/dts/zynq-7000.dtsi
 index fc90f47f9c03..2690e6d01f71 100644
 --- a/arch/arm/boot/dts/zynq-7000.dtsi
 +++ b/arch/arm/boot/dts/zynq-7000.dtsi
 @@ -145,7 +145,7 @@
   cache-level = 2;
   };
 
 - memory-controller@f8006000 {
 + mc: memory-controller@f8006000 {
same here. 'mc0', there are definitely options to add more memory
controllers to Zynq in the PL. Also, I'd spell it out instead of
abbreviating the name.

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

Re: [PATCH 1/2] ARM: zynq: DT: Add missing reference for ADC

[Sören Brinkmann]

On Wed, 2014-09-24 at 07:06PM +0200, Michal Simek wrote:
 On 09/24/2014 06:18 PM, Sören Brinkmann wrote:
  On Wed, 2014-09-24 at 04:01PM +0200, Michal Simek wrote:
  Add missing reference for ADC node.
 
  Signed-off-by: Michal Simek michal.si...@xilinx.com
  ---
 
   arch/arm/boot/dts/zynq-7000.dtsi | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)
 
  diff --git a/arch/arm/boot/dts/zynq-7000.dtsi 
  b/arch/arm/boot/dts/zynq-7000.dtsi
  index 772381fe07bb..fc90f47f9c03 100644
  --- a/arch/arm/boot/dts/zynq-7000.dtsi
  +++ b/arch/arm/boot/dts/zynq-7000.dtsi
  @@ -64,7 +64,7 @@
 interrupt-parent = intc;
 ranges;
 
  -  adc@f8007100 {
  +  adc: adc@f8007100 {
  I think we enumerated all labels. I.e. 'adc0' is probably better.
 
 I was thinking about it and the reason I didn't use that adc0 was
 that it is unique and it is just one in PS. Maybe there could be another adc 
 in PL
 but it will be probably automated not to use adc key world.
 For PL part names was depending on user description.
 
 Anyway logic was not to use enumerated label for unique IPs.
As you said yourself, due to having/being an FPGA and also the
extensibility of the platform via buses (including PCIe), I
would not consider any IP to be unique.

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

Re: [PATCH v6 07/12] usb: chipidea: add a usb2 driver for ci13xxx

[Sören Brinkmann]

On Tue, 2014-09-23 at 12:28PM +0200, Antoine Tenart wrote:
 Add a USB2 ChipIdea driver for ci13xxx, with optional PHY, clock
 and DMA mask, to support USB2 ChipIdea controllers that don't need
 specific functions.
 
 Tested on the Marvell Berlin SoCs USB controllers.
 
 Signed-off-by: Antoine Tenart antoine.ten...@free-electrons.com

This driver also works for Zynq. I didn't do extensive testing, but I
could access a flash drive successfully.

Tested-by: Soren Brinkmann soren.brinkm...@xilinx.com

Thanks,
Sören
--
To unsubscribe from this list: send the line unsubscribe devicetree in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] ARM: zynq: DT: Add Ethernet phys

[Sören Brinkmann]

On Fri, 2014-08-29 at 05:18PM +0200, Andreas Färber wrote:
 Am 29.08.2014 16:08, schrieb Michal Simek:
  On 08/25/2014 10:21 PM, Florian Fainelli wrote:
  On 08/25/2014 10:46 AM, Jason Gunthorpe wrote:
  On Fri, Aug 22, 2014 at 01:47:09PM -0700, Florian Fainelli wrote:
 
   - the ID based strings seem to be not needed since, IIUC, the core
 reads the ID from the PHY and uses it, so I just left it out not
 trying to figure out how to obtain the correct ID
 
  It is not needed, but it is one way to specify a PHY device if you do
  not know what compatible string to use instead.
 
  No, it is a way to specify a PHY device if the kernel can't auto probe
  the Phy ID.
 
  Last I checked, the kernel doesn't support plain text compatible
  strings for phys - everything is driven on the phy id, either auto
  probed or specified in the DT.
 
  That's right. Some PHY drivers might be relying on specific compatible
  strings though, but not the core PHY library that probes and maps a
  driver to a PHY node.
 
 
   - the marvell compatible strings are used in our vendor tree. They
 aren't used anywhere but in our vendor tree. I though keeping them is
 nice since it identifies the PHY fully. And in case that level of
 detail is needed at some point it is already there.
 
  And this is the recommended way to do it in case we ever need to key a
  software decision based on the hardware.
 
  All compatible strings need to be documented.
 
  .. and they need to encode more information than you get from the phy
  id - die revsision, package option, functional options, voltage
  codes. Etc.
 
  .. and they actually need to be *right*
 
  Agreed.
 
 
  An example: The kernel reports 88E1318S for all four chips in that
  family, AFAIK you have to read the package marking to figure out which
  you have (it is the same die, with options switched on/off at
  packaging time). People have already posted patches trying to
  helpfully add a 'marvell,88E1318S' compatible string based on kernel
  output. Except it is wrong, it isn't actually the '8S version in the
  HW.
 
  Even worse, Marvell has a whole series of socket compatible phys. Just
  because the board the DT author looked at has a '318, doesn't mean
  that every board ever made will. We've actually already been switching
  between the 318 and 318S for production depending on which has part
  availability.
 
  Basically: don't try to override self-discoverable hardware in DT
  without a really good reason.
 
  I think that's a very good point, at the very least let's use a
  compatible string that contains the full 32-bits PHY OUI.
  
  I think resolution is:
  1. Do not use marvell,88e1518 because it is not listed anywhere
  2. Do not add ethernet-phy-id. because it breaks autodetection
  if there is different phy on the board and we shouldn't restrict us in this.
  In spite of autodetection takes some time.
  3. ethernet-phy-ieee802.3-c22 is optional that's why doesn't need to be 
  added
  4. Any listed compatible string has to be parsed which takes time
  
  That's why I think make sense not to use any compatible string.
  This should give us all flexibility which we want to have.
 
 Sorry, but we do need some node that we can reference via phy-handle
 from the gem node, don't we?
 
 In that case, is not specifying any compatible string a valid option?
 
 If you don't want to specify the IDs, then I would've assumed we need to
 specify the -c22 in order to have *some* compatible string in order to
 trigger loading of the appropriate driver module.

The compatible string is listed as optional property for PHYs. So, not
having one is an option, I guess. But, I'd also prefer to at least keep
the -c22 one, since I saw problems when I tried using -c45 (the Zed phy
should support it...).
Also, so far, we haven't had any phy nodes in our Zynq dts files and
Ethernet worked, so the auto-detection there works pretty well
apparently. But it may be problematic if more than a single PHY is on
the MDIO bus, I'd assume.

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

Re: [PATCH] ARM: zynq: DT: Add LEDs to zc702 DT

[Sören Brinkmann]

On Thu, 2014-08-28 at 09:22AM -0700, Soren Brinkmann wrote:
 From: Ezra Savard ezra.sav...@xilinx.com
 
 Adds LEDs to the zc702 devicetree for use with the leds-gpio driver.
 
 Signed-off-by: Ezra Savard ezra.sav...@xilinx.com
 Reviewed-by: Soren Brinkmann soren.brinkm...@xilinx.com
 ---
  arch/arm/boot/dts/zynq-zc702.dts | 10 ++
  1 file changed, 10 insertions(+)
 
 diff --git a/arch/arm/boot/dts/zynq-zc702.dts 
 b/arch/arm/boot/dts/zynq-zc702.dts
 index 835c3089c61c..4e9fe26b3e85 100644
 --- a/arch/arm/boot/dts/zynq-zc702.dts
 +++ b/arch/arm/boot/dts/zynq-zc702.dts
 @@ -27,6 +27,16 @@
   bootargs = console=ttyPS0,115200 earlyprintk;
   };
  
 + gpio-leds {
The node name should rather be just 'leds'. That slipped through, sorry.
Do you want a v2 for that?

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

Re: [PATCH 3/3] ARM: zynq: DT: Add Ethernet phys

[Sören Brinkmann]

On Fri, 2014-08-22 at 10:20AM -0600, Jason Gunthorpe wrote:
 On Thu, Aug 21, 2014 at 08:49:19AM -0700, Sören Brinkmann wrote:
 
  So my thinkings:
   - the compatible string with the -c22 is used and documented in the phy
 bindings, it should be there.
 
 Yes
 
   - the ID based strings seem to be not needed since, IIUC, the core
 reads the ID from the PHY and uses it, so I just left it out not
 trying to figure out how to obtain the correct ID
 
 It is certainly optional, I added the property to solve some obscure
 problems with probing, but I've noticed people using it more broadly.
 I suspect it also speeds up booting a tiny bit.
 
   - the marvell compatible strings are used in our vendor tree. They
 aren't used anywhere but in our vendor tree. I though keeping them is
 nice since it identifies the PHY fully. And in case that level of
 detail is needed at some point it is already there.
 
 DT is complex enough already, don't include useless compatible strings
 in mainline - people will see and tend to blindly copy. Ideally your
 vendor tree would use the standard property :|

There are different opinions on this. I also heard to just add such
strings, so in case this level of differentiation becomes necessary at
some later point, it is already existing.

Sören

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

Re: [PATCH 3/3] ARM: zynq: DT: Add Ethernet phys

[Sören Brinkmann]

On Thu, 2014-08-21 at 01:32PM +0200, Andreas Färber wrote:
 Am 21.08.2014 10:41, schrieb Michal Simek:
  On 08/20/2014 05:56 PM, Soren Brinkmann wrote:
  Add missing Ethernet phys to Zynq DTs.
 
  Signed-off-by: Soren Brinkmann soren.brinkm...@xilinx.com
  ---
   arch/arm/boot/dts/zynq-zc702.dts | 6 ++
   arch/arm/boot/dts/zynq-zc706.dts | 6 ++
   arch/arm/boot/dts/zynq-zed.dts   | 6 ++
   3 files changed, 18 insertions(+)
 
  diff --git a/arch/arm/boot/dts/zynq-zc702.dts 
  b/arch/arm/boot/dts/zynq-zc702.dts
  index 30bcfe20f0bc..fa810505ab8f 100644
  --- a/arch/arm/boot/dts/zynq-zc702.dts
  +++ b/arch/arm/boot/dts/zynq-zc702.dts
  @@ -36,6 +36,12 @@
   gem0 {
 status = okay;
 phy-mode = rgmii-id;
  +  phy-handle = ethernet_phy;
  +
  +  ethernet_phy: ethernet-phy@7 {
  +  compatible = marvell,88e1116r, ethernet-phy-ieee802.3-c22;
  
  c22 is completely unused by the kernel and also c22 is default option 
  anyway.
  Any advantage to have c22 specified here?
 
 http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/net/phy.txt
 
 Sören's marvell,* entries do not seem documented, therefore I used the
 documented ethernet-phy-id. based syntax. The documented
 example specifically uses -c22, too.
 
 Either it's okay to prepend unrecognized model strings, then you should
 update zynq-parallella.dts as well (I put the model in a comment there)
 or use the official strings like I used and keep the readable models as
 comments. Documenting all those marvell,88e1116r, marvell,88e1518,
 marvell,88e1318 PHY bindings and possibly prepending them to the ID
 based strings would be another option, of course.

These phy-bindings are everything than obvious to me. It seems the docs
are spread across a couple of different files and not fully up to date
either. I basically tried to get something working out of the docs, the
parallela and our vendor DT files.

So my thinkings:
 - the compatible string with the -c22 is used and documented in the phy
   bindings, it should be there.
 - the ID based strings seem to be not needed since, IIUC, the core
   reads the ID from the PHY and uses it, so I just left it out not
   trying to figure out how to obtain the correct ID
 - the marvell compatible strings are used in our vendor tree. They
   aren't used anywhere but in our vendor tree. I though keeping them is
   nice since it identifies the PHY fully. And in case that level of
   detail is needed at some point it is already there.

Assuming that we wanna keep things this way, I'm happy to re-spin this
patch and also add a similar compatible string to the parallela DT.

Thanks,
Sören

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

Re: [PATCH 1/9] ARM: zynq: PM: Enable A9 internal clock gating feature

[Sören Brinkmann]

On Thu, 2014-08-21 at 02:19PM +0200, Michal Simek wrote:
 On 08/20/2014 10:41 PM, Soren Brinkmann wrote:
  Signed-off-by: Soren Brinkmann soren.brinkm...@xilinx.com
  ---
   arch/arm/mach-zynq/common.c  |  6 ++
   arch/arm/mach-zynq/common.h  | 11 +++
   arch/arm/mach-zynq/platsmp.c |  9 +
   3 files changed, 26 insertions(+)
  
  diff --git a/arch/arm/mach-zynq/common.c b/arch/arm/mach-zynq/common.c
  index 31a6fa40ba37..3cb7c198615a 100644
  --- a/arch/arm/mach-zynq/common.c
  +++ b/arch/arm/mach-zynq/common.c
  @@ -98,6 +98,11 @@ static int __init zynq_get_revision(void)
  return revision;
   }
   
  +static void __init zynq_init_late(void)
  +{
  +   zynq_core_pm_init();
  +}
  +
   /**
* zynq_init_machine - System specific initialization, intended to be
*called from board specific initialization.
  @@ -204,6 +209,7 @@ DT_MACHINE_START(XILINX_EP107, Xilinx Zynq Platform)
  .map_io = zynq_map_io,
  .init_irq   = zynq_irq_init,
  .init_machine   = zynq_init_machine,
  +   .init_late  = zynq_init_late,
  .init_time  = zynq_timer_init,
  .dt_compat  = zynq_dt_match,
  .reserve= zynq_memory_init,
  diff --git a/arch/arm/mach-zynq/common.h b/arch/arm/mach-zynq/common.h
  index f652f0a884a6..596ef0b5067c 100644
  --- a/arch/arm/mach-zynq/common.h
  +++ b/arch/arm/mach-zynq/common.h
  @@ -40,4 +40,15 @@ extern void __iomem *zynq_scu_base;
   /* Hotplug */
   extern void zynq_platform_cpu_die(unsigned int cpu);
   
  +static inline void zynq_core_pm_init(void)
  +{
  +   /* A9 clock gating */
  +   asm volatile (mrc  p15, 0, r12, c15, c0, 0\n
  + orr  r12, r12, #1\n
  + mcr  p15, 0, r12, c15, c0, 0\n
  + : /* no outputs */
  + : /* no inputs */
  + : r12);
  +}
  +
   #endif
  diff --git a/arch/arm/mach-zynq/platsmp.c b/arch/arm/mach-zynq/platsmp.c
  index abc82ef085c1..616b99e07c60 100644
  --- a/arch/arm/mach-zynq/platsmp.c
  +++ b/arch/arm/mach-zynq/platsmp.c
  @@ -112,6 +112,14 @@ static void __init zynq_smp_prepare_cpus(unsigned int 
  max_cpus)
  scu_enable(zynq_scu_base);
   }
   
  +/*
  + * This function is in the hotplug path. Don't move it into the init 
  section!!
  + */
 
 Worth to use kernel-doc format here.

The secondary init function is kind of documented in
arch/arm/include/asm/smp.h as part of the smp_operations struct. Do you
want more documentation here?

Sören

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

Re: [PATCH 4/9] ARM: zynq: PM: Enable DDR self-refresh and clock stop

[Sören Brinkmann]

On Thu, 2014-08-21 at 02:25PM +0200, Michal Simek wrote:
 On 08/20/2014 10:41 PM, Soren Brinkmann wrote:
  The DDR controller can detect idle periods and leverage low power
  features like self-refresh and clock stop.
  When new requests occur, the DDRC resumes normal operation.
  
  Signed-off-by: Soren Brinkmann soren.brinkm...@xilinx.com
  ---
   arch/arm/mach-zynq/Makefile |  2 +-
   arch/arm/mach-zynq/common.c |  1 +
   arch/arm/mach-zynq/common.h |  2 ++
   arch/arm/mach-zynq/pm.c | 84 
  +
   4 files changed, 88 insertions(+), 1 deletion(-)
   create mode 100644 arch/arm/mach-zynq/pm.c
  
  diff --git a/arch/arm/mach-zynq/Makefile b/arch/arm/mach-zynq/Makefile
  index 1b25d92ebf22..820dff6e1eba 100644
  --- a/arch/arm/mach-zynq/Makefile
  +++ b/arch/arm/mach-zynq/Makefile
  @@ -3,7 +3,7 @@
   #
   
   # Common support
  -obj-y  := common.o slcr.o
  +obj-y  := common.o slcr.o pm.o
   CFLAGS_REMOVE_hotplug.o=-march=armv6k
   CFLAGS_hotplug.o   =-Wa,-march=armv7-a -mcpu=cortex-a9
   obj-$(CONFIG_HOTPLUG_CPU)  += hotplug.o
  diff --git a/arch/arm/mach-zynq/common.c b/arch/arm/mach-zynq/common.c
  index 3cb7c198615a..6bd13e5ce6b7 100644
  --- a/arch/arm/mach-zynq/common.c
  +++ b/arch/arm/mach-zynq/common.c
  @@ -101,6 +101,7 @@ static int __init zynq_get_revision(void)
   static void __init zynq_init_late(void)
   {
  zynq_core_pm_init();
  +   zynq_pm_late_init();
   }
   
   /**
  diff --git a/arch/arm/mach-zynq/common.h b/arch/arm/mach-zynq/common.h
  index 596ef0b5067c..87945fa2a179 100644
  --- a/arch/arm/mach-zynq/common.h
  +++ b/arch/arm/mach-zynq/common.h
  @@ -40,6 +40,8 @@ extern void __iomem *zynq_scu_base;
   /* Hotplug */
   extern void zynq_platform_cpu_die(unsigned int cpu);
   
  +int zynq_pm_late_init(void);
  +
   static inline void zynq_core_pm_init(void)
   {
  /* A9 clock gating */
  diff --git a/arch/arm/mach-zynq/pm.c b/arch/arm/mach-zynq/pm.c
  new file mode 100644
  index ..19955917aac8
  --- /dev/null
  +++ b/arch/arm/mach-zynq/pm.c
  @@ -0,0 +1,84 @@
  +/*
  + * Zynq power management
  + *
  + *  Copyright (C) 2012 - 2014 Xilinx
  + *
  + *  Sören Brinkmann soren.brinkm...@xilinx.com
  + *
  + * This program 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 program 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.
  + *
  + * You should have received a copy of the GNU General Public License
  + * along with this program.  If not, see http://www.gnu.org/licenses/.
 
 IRC this paragraph is not necessary and can be just removed.
It's the standard header as suggested by gnu.org.

 
  + */
  +
  +#include linux/io.h
  +#include linux/of_address.h
  +#include linux/of_device.h
  +#include common.h
  +
  +/* register offsets */
  +#define DDRC_CTRL_REG1_OFFS0x60
  +#define DDRC_DRAM_PARAM_REG3_OFFS  0x20
  +
  +/* bitfields */
  +#define DDRC_CLOCKSTOP_MASKBIT(23)
  +#define DDRC_SELFREFRESH_MASK  BIT(12)
  +
  +static void __iomem *ddrc_base;
  +
  +/**
  + * zynq_pm_ioremap() - Create IO mappings
  + * @comp:  DT compatible string
  + * Returns a pointer to the mapped memory or NULL.
  + *
  + * Remap the memory region for a compatible DT node.
  + */
 
 [linux]$ ./scripts/kernel-doc -man -v arch/arm/mach-zynq/pm.c  /dev/null
 Info(arch/arm/mach-zynq/pm.c:38): Scanning doc for
 Warning(arch/arm/mach-zynq/pm.c:45): No description found for return value of 
 'zynq_pm_ioremap'
I'll fix that.

 
  +static void __iomem *zynq_pm_ioremap(const char *comp)
  +{
  +   struct device_node *np;
  +   void __iomem *base = NULL;
  +
  +   np = of_find_compatible_node(NULL, NULL, comp);
  +   if (np) {
  +   base = of_iomap(np, 0);
  +   of_node_put(np);
  +   } else {
  +   pr_warn(%s: no compatible node found for '%s'\n, __func__,
  +   comp);
  +   }
  +
  +   return base;
  +}
  +
  +int __init zynq_pm_late_init(void)
 
 kernel doc will be nice to have here.
I'll add a line or two.

 
  +{
  +   u32 reg;
  +
  +   ddrc_base = zynq_pm_ioremap(xlnx,zynq-ddrc-a05);
  +   if (!ddrc_base) {
  +   pr_warn(%s: Unable to map DDRC IO memory.\n, __func__);
  +   } else {
  +   /*
  +* Enable DDRC self-refresh and clock stop features. The HW
  +* takes care of entering/exiting the correct modes depending
  +* on activity state.
  +*/
  +   reg = readl(ddrc_base + DDRC_CTRL_REG1_OFFS);
  +   reg

Re: [PATCH 9/9] ARM: zynq: Rename 'zynq_platform_cpu_die'

[Sören Brinkmann]

On Thu, 2014-08-21 at 02:28PM +0200, Michal Simek wrote:
 On 08/20/2014 10:41 PM, Soren Brinkmann wrote:
  Match the naming pattern of all other SMP ops and rename
  zynq_platform_cpu_die -- zynq_cpu_die.
  
  Signed-off-by: Soren Brinkmann soren.brinkm...@xilinx.com
  ---
   arch/arm/mach-zynq/platsmp.c | 4 ++--
   1 file changed, 2 insertions(+), 2 deletions(-)
  
  diff --git a/arch/arm/mach-zynq/platsmp.c b/arch/arm/mach-zynq/platsmp.c
  index 04e578718aa2..95933c5e70e1 100644
  --- a/arch/arm/mach-zynq/platsmp.c
  +++ b/arch/arm/mach-zynq/platsmp.c
  @@ -138,7 +138,7 @@ static int zynq_cpu_kill(unsigned cpu)
*
* Called with IRQs disabled
*/
  -static void zynq_platform_cpu_die(unsigned int cpu)
  +static void zynq_cpu_die(unsigned int cpu)
   {
  zynq_slcr_cpu_state_write(cpu, true);
   
  @@ -158,7 +158,7 @@ struct smp_operations zynq_smp_ops __initdata = {
  .smp_boot_secondary = zynq_boot_secondary,
  .smp_secondary_init = zynq_secondary_init,
   #ifdef CONFIG_HOTPLUG_CPU
  -   .cpu_die= zynq_platform_cpu_die,
  +   .cpu_die= zynq_cpu_die,
  .cpu_kill   = zynq_cpu_kill,
   #endif
   };
 
 Will be good if you can move fix that kernel-doc format for this function
 too. It is just nice to have thing.

All these SMP-ops should be documented in the header defining that
struct, shouldn't they?

Sören

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

Re: [PATCH 8/9] ARM: zynq: Remove hotplug.c

[Sören Brinkmann]

On Thu, 2014-08-21 at 03:32AM +0200, Daniel Lezcano wrote:
 On 08/20/2014 10:41 PM, Soren Brinkmann wrote:
 The hotplug code contains only a single function, which is an SMP
 function. Move that to platsmp.c where all other SMP runctions reside.
 That allows removing hotplug.c and declaring the cpu_die function
 static.
 
 Signed-off-by: Soren Brinkmann soren.brinkm...@xilinx.com
 ---
   arch/arm/mach-zynq/Makefile  |  1 -
   arch/arm/mach-zynq/common.h  |  3 +--
   arch/arm/mach-zynq/hotplug.c | 17 -
   arch/arm/mach-zynq/platsmp.c | 18 ++
   4 files changed, 19 insertions(+), 20 deletions(-)
 
 diff --git a/arch/arm/mach-zynq/Makefile b/arch/arm/mach-zynq/Makefile
 index 820dff6e1eba..c85fb3f7d5cd 100644
 --- a/arch/arm/mach-zynq/Makefile
 +++ b/arch/arm/mach-zynq/Makefile
 @@ -6,5 +6,4 @@
   obj-y  := common.o slcr.o pm.o
   CFLAGS_REMOVE_hotplug.o=-march=armv6k
   CFLAGS_hotplug.o   =-Wa,-march=armv7-a -mcpu=cortex-a9
 -obj-$(CONFIG_HOTPLUG_CPU)   += hotplug.o
   obj-$(CONFIG_SMP)  += headsmp.o platsmp.o
 diff --git a/arch/arm/mach-zynq/common.h b/arch/arm/mach-zynq/common.h
 index c0773e87e83c..e6bb12c50a23 100644
 --- a/arch/arm/mach-zynq/common.h
 +++ b/arch/arm/mach-zynq/common.h
 @@ -39,8 +39,7 @@ extern struct smp_operations zynq_smp_ops __initdata;
 
   extern void __iomem *zynq_scu_base;
 
 -/* Hotplug */
 -extern void zynq_platform_cpu_die(unsigned int cpu);
 +int zynq_pm_late_init(void);
 
   int zynq_pm_late_init(void);
 
 diff --git a/arch/arm/mach-zynq/hotplug.c b/arch/arm/mach-zynq/hotplug.c
 index fe44a05677e2..b685c89f11e4 100644
 --- a/arch/arm/mach-zynq/hotplug.c
 +++ b/arch/arm/mach-zynq/hotplug.c
 @@ -12,20 +12,3 @@
*/
   #include asm/proc-fns.h
 
 -/*
 - * platform-specific code to shutdown a CPU
 - *
 - * Called with IRQs disabled
 - */
 -void zynq_platform_cpu_die(unsigned int cpu)
 -{
 -zynq_slcr_cpu_state_write(cpu, true);
 -
 -/*
 - * there is no power-control hardware on this platform, so all
 - * we can do is put the core into WFI; this is safe as the calling
 - * code will have already disabled interrupts
 - */
 -for (;;)
 -cpu_do_idle();
 -}
 diff --git a/arch/arm/mach-zynq/platsmp.c b/arch/arm/mach-zynq/platsmp.c
 index f77f7ca4c45b..04e578718aa2 100644
 --- a/arch/arm/mach-zynq/platsmp.c
 +++ b/arch/arm/mach-zynq/platsmp.c
 @@ -132,6 +132,24 @@ static int zynq_cpu_kill(unsigned cpu)
  zynq_slcr_cpu_stop(cpu);
  return 1;
   }
 +
 +/*
 + * platform-specific code to shutdown a CPU
 + *
 + * Called with IRQs disabled
 + */
 +static void zynq_platform_cpu_die(unsigned int cpu)
 +{
 +zynq_slcr_cpu_state_write(cpu, true);
 +
 +/*
 + * there is no power-control hardware on this platform, so all
 + * we can do is put the core into WFI; this is safe as the calling
 + * code will have already disabled interrupts
 + */
 +for (;;)
 +cpu_do_idle();
 
 IIUC, the cpu_do_idle() will flush the L1 cache and then call the
 WFI. It makes sense if we are about to power down the core. So I am
 wondering if we can just call wfi() instead.

I'm not sure - it's not that trivial to trace that through the sources.
But I think cpu_do_idle ends up in cpu_v7_do_idle which is just:
ENTRY(cpu_v7_do_idle)   
 
dsb @ WFI may enter a 
low-power mode 
wfi 
 
ret lr  
 
ENDPROC(cpu_v7_do_idle)

I think that is what we want here.

Thanks,
Sören

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

Re: [PATCH v3] ARM: zynq: DT: Add CAN node

[Sören Brinkmann]

On Mon, 2014-07-28 at 11:17AM +0200, Michal Simek wrote:
 Add node describing Zynq's CAN controller.
 
 Signed-off-by: Michal Simek michal.si...@xilinx.com
Reviewed-by: Soren Brinkmann soren.brinkm...@xilinx.com

Sören

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

Re: [PATCH v3] ARM: dts: zynq: Add SPI

[Sören Brinkmann]

On Fri, 2014-07-25 at 01:12PM +0200, Andreas Färber wrote:
 Signed-off-by: Andreas Färber afaer...@suse.de
Reviewed-by: Soren Brinkmann soren.brinkm...@xilinx.com

Sören

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

Re: [PATCH v2 06/11] ARM: dts: zynq: Add DMAC for Parallella

[Sören Brinkmann]

On Fri, 2014-07-25 at 10:24AM +0200, Andreas Färber wrote:
 Am 25.07.2014 10:02, schrieb Michal Simek:
  On 07/25/2014 01:28 AM, Sören Brinkmann wrote:
  On Fri, 2014-07-25 at 01:00AM +0200, Andreas Färber wrote:
  Signed-off-by: Andreas Färber afaer...@suse.de
  ---
   v2: New
   
   arch/arm/boot/dts/zynq-7000.dtsi  | 17 +
   arch/arm/boot/dts/zynq-parallella.dts |  4 
   2 files changed, 21 insertions(+)
 
  diff --git a/arch/arm/boot/dts/zynq-7000.dtsi 
  b/arch/arm/boot/dts/zynq-7000.dtsi
  index eed3df0..1a70277 100644
  --- a/arch/arm/boot/dts/zynq-7000.dtsi
  +++ b/arch/arm/boot/dts/zynq-7000.dtsi
  @@ -223,6 +223,23 @@
};
};
   
  + dmac_s: dmac@f8003000 {
  + compatible = arm,pl330, arm,primecell;
  + reg = 0xf8003000 0x1000;
  + status = disabled;
  I think for this IP we can omit the 'status' property since it is always
  enabled. I don't see a reason to override it in each board DT.
  
  Done this change myself
 
 Fine with me, but allow me to point out that the TRM documents the DMAC
 being mapped as DMAC S at the above address, and as DMAC NS at F800_4000
 (secure vs. non-secure, ch. 4.6, p. 116). Not sure how this would be
 handled driver-wise if not through alternative dt nodes?

The upstream Linux runs in secure state on Zynq, hence I think this is fine. If
somebody wants to run Linux on Zynq non-secure they have to do some work
anyhow. This way the standard configuration can use the DMA engine.

Sören

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

Re: [PATCH v2 08/11] ARM: dts: zynq: Add VDMA to Parallella

[Sören Brinkmann]

On Fri, 2014-07-25 at 11:47AM +0200, Andreas Färber wrote:
 Hi Sören,
 
 Am 25.07.2014 01:22, schrieb Sören Brinkmann:
  On Fri, 2014-07-25 at 01:00AM +0200, Andreas Färber wrote:
  Signed-off-by: Andreas Färber afaer...@suse.de
  ---
   v2: New
   
   arch/arm/boot/dts/zynq-parallella.dts | 25 +
   1 file changed, 25 insertions(+)
 
  diff --git a/arch/arm/boot/dts/zynq-parallella.dts 
  b/arch/arm/boot/dts/zynq-parallella.dts
  index e60a0a9..8beaacc 100644
  --- a/arch/arm/boot/dts/zynq-parallella.dts
  +++ b/arch/arm/boot/dts/zynq-parallella.dts
  @@ -32,6 +32,31 @@
 bootargs = console=ttyPS0,115200 earlyprintk 
  root=/dev/mmcblk0p2 rootfstype=ext4 rw rootwait;
 linux,stdout-path = /amba/serial@e0001000;
 };
  +
  +  fpga {
  Do you really want FPGA components in this DT?
  If somebody tries booting with this DT without programming with a
  corresponding bitstream, the whole system might hang.
  Just something to consider.
 
 Well, that's related to a question that remained unanswered on v1:
 whether we may need to turn this into a .dtsi to cope with variations.
 
 The Parallella has an on-board µHDMI connector, and two bitstreams are
 delivered - one for HDMI and one for headless usage. In my testing I am
 using the original HDMI bitstream but serial console for lack of
 upstream HDMI drivers. Do you think we need to provide
 zynq-parallella-hdmi.dts and zynq-parallella-headless.dts? (It gets
 worse if at some point we need to handle variations of the on-board
 Epiphany chip plus the bitstreams - at least the Z7010 vs. Z7020 doesn't
 affect DT AFAICT.)
 
 Since, as noted in the cover letter, these FPGA patches are not yet
 fully testable, I wouldn't mind deferring them, but wanted to get them
 out for review early.
 
 http://www.parallella.org/2014/07/14/new-parallella-product-offerings/
 indicates there will be a new variation in gen2 without USB/HDMI. Would
 it be valid to #include a .dts (rather than .dtsi) to override status
 and keep number of Parallella files low?

For these FPGA devices, it would be nice if it was possible to include
a dts file. I think there were problems with that, but don't know what
the current status of that is. Then it would be possible to have the SOC
dtsi a board dts and systems with FPGA components can include the board
dts.
I also heard that some people are getting upset with all the includes
going on in the dts files.

And the next question would be: How many dts files do you really want
in the upstream kernel tree? Since we deal with FPGAs, there is
virtually and infinite number of different systems that you could
create relatively easily.

Sören

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

Re: [PATCH v2] ARM: zynq: DT: Add CAN node

[Sören Brinkmann]

On Fri, 2014-07-25 at 08:52AM +0200, Michal Simek wrote:
 Add node describing Zynq's CAN controller.
 
 Signed-off-by: Michal Simek michal.si...@xilinx.com
 ---
 
 Changes in v2:
 - Add can1
 - Enable can0 for zc702
 - Use status property
 
  arch/arm/boot/dts/zynq-7000.dtsi | 26 +-
  arch/arm/boot/dts/zynq-zc702.dts |  4 
  2 files changed, 29 insertions(+), 1 deletion(-)
 
 diff --git a/arch/arm/boot/dts/zynq-7000.dtsi 
 b/arch/arm/boot/dts/zynq-7000.dtsi
 index 366ca6434f54..983148111e3a 100644
 --- a/arch/arm/boot/dts/zynq-7000.dtsi
 +++ b/arch/arm/boot/dts/zynq-7000.dtsi
 @@ -71,7 +71,31 @@
   interrupts = 0 7 4;
   interrupt-parent = intc;
   clocks = clkc 12;
 - };
 + };
 +
 + can0: can@e0008000 {
 + compatible = xlnx,zynq-can-1.0;
 + status = disabled;
 + clocks = clkc 19, clkc 36;
 + clock-names = can_clk, pclk;
 + reg = 0xe0008000 0x1000;
 + interrupts = 0 28 4;
 + interrupt-parent = intc;
 + tx-fifo-depth = 0x40;
 + rx-fifo-depth = 0x40;
 + };
 +
 + can1: can@e0009000 {
 + compatible = xlnx,zynq-can-1.0;
 + status = disabled;
 + clocks = clkc 19, clkc 36;
The clocks for CAN1 must be outputs 20 and 37 of the clkc.

Sören

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

Re: [PATCH] ARM: zynq: DT: Add CAN node

[Sören Brinkmann]

On Thu, 2014-07-24 at 08:44AM +0200, Michal Simek wrote:
 On 07/23/2014 06:07 PM, Sören Brinkmann wrote:
  On Wed, 2014-07-23 at 03:05PM +0200, Michal Simek wrote:
  Add node describing Zynq's CAN controller.
 
  Signed-off-by: Michal Simek michal.si...@xilinx.com
  ---
 
   arch/arm/boot/dts/zynq-7000.dtsi | 13 -
   1 file changed, 12 insertions(+), 1 deletion(-)
 
  diff --git a/arch/arm/boot/dts/zynq-7000.dtsi 
  b/arch/arm/boot/dts/zynq-7000.dtsi
  index 366ca6434f54..2287d9b4ed1a 100644
  --- a/arch/arm/boot/dts/zynq-7000.dtsi
  +++ b/arch/arm/boot/dts/zynq-7000.dtsi
  @@ -71,7 +71,18 @@
 interrupts = 0 7 4;
 interrupt-parent = intc;
 clocks = clkc 12;
  -  };
  +  };
  +
  +  can0: can@e0008000 {
  +  compatible = xlnx,zynq-can-1.0;
  +  clocks = clkc 19, clkc 36;
  +  clock-names = can_clk, pclk;
  +  reg = 0xe0008000 0x1000;
  +  interrupts = 0 28 4;
  +  interrupt-parent = intc;
  +  tx-fifo-depth = 0x40;
  +  rx-fifo-depth = 0x40;
  +  };
  
  What about the second CAN core? You also probably want to add
  'status = disabled' in the dtsi.
 
 Second can core - yes.
 
 I can add status = disabled but then question is if make
 sense to change status for any zynq dts file. Or just
 keep enable both.
 
 In our repo is enabled can0 on zc702.
 
 What do you think?

Well, it should be enabled on boards that have CAN pinned out. Does any
of our boards have that?

Sören

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

Re: [PATCH v2 04/11] ARM: dts: zynq: Update deprecated xuartps clock names

[Sören Brinkmann]

On Fri, 2014-07-25 at 01:00AM +0200, Andreas Färber wrote:
 Avoids deprecation warning messages.

A patch that updates those names is already in armsoc.

Sören

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

Re: [PATCH v2 05/11] ARM: dts: zynq: Add QSPI for Parallella

[Sören Brinkmann]

On Fri, 2014-07-25 at 01:00AM +0200, Andreas Färber wrote:
 Prepare SPI0 and SPI1 while at it.
 
 Signed-off-by: Andreas Färber afaer...@suse.de
 ---
  v2: New
  
  arch/arm/boot/dts/zynq-7000.dtsi  | 37 
 +++
  arch/arm/boot/dts/zynq-parallella.dts |  4 
  2 files changed, 41 insertions(+)
 
 diff --git a/arch/arm/boot/dts/zynq-7000.dtsi 
 b/arch/arm/boot/dts/zynq-7000.dtsi
 index 8fd826a..eed3df0 100644
 --- a/arch/arm/boot/dts/zynq-7000.dtsi
 +++ b/arch/arm/boot/dts/zynq-7000.dtsi
 @@ -122,6 +122,30 @@
   interrupts = 0 50 4;
   };
  
 + spi0: spi@e0006000 {
 + compatible = xlnx,zynq-spi-r1p6;
 + reg = 0xe0006000 0x1000;
 + status = disabled;
 + interrupt-parent = intc;
 + interrupts = 0 26 4;
 + clocks = clkc 25, clkc 34;
 + clock-names = ref_clk, pclk;
 + #address-cells = 1;
 + #size-cells = 0;
 + };
 +
 + spi1: spi@e0007000 {
 + compatible = xlnx,zynq-spi-r1p6;
 + reg = 0xe0007000 0x1000;
 + status = disabled;
 + interrupt-parent = intc;
 + interrupts = 0 49 4;
 + clocks = clkc 26, clkc 35;
 + clock-names = ref_clk, pclk;
 + #address-cells = 1;
 + #size-cells = 0;
 + };
 +
Until here things look good.

   gem0: ethernet@e000b000 {
   compatible = cdns,gem;
   reg = 0xe000b000 0x4000;
 @@ -140,6 +164,19 @@
   clock-names = pclk, hclk, tx_clk;
   };
  
 + qspi: qspi@e000d000 {
 + compatible = xlnx,zynq-spi-r1p6;
 + reg = 0xe000d000 0x1000;
 + status = disabled;
 + interrupt-parent = intc;
 + interrupts = 0 19 4;
 + clocks = clkc 10, clkc 43;
 + clock-names = ref_clk, pclk;
 + num-cs = 1;
 + #address-cells = 1;
 + #size-cells = 0;
 + };
 +
I'm not sure what the status of this driver is. I think QSPI is still
under review on the mailing lists and I don't think we should add this
yet.

Sören

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

Re: [PATCH v2 08/11] ARM: dts: zynq: Add VDMA to Parallella

[Sören Brinkmann]

On Fri, 2014-07-25 at 01:00AM +0200, Andreas Färber wrote:
 Signed-off-by: Andreas Färber afaer...@suse.de
 ---
  v2: New
  
  arch/arm/boot/dts/zynq-parallella.dts | 25 +
  1 file changed, 25 insertions(+)
 
 diff --git a/arch/arm/boot/dts/zynq-parallella.dts 
 b/arch/arm/boot/dts/zynq-parallella.dts
 index e60a0a9..8beaacc 100644
 --- a/arch/arm/boot/dts/zynq-parallella.dts
 +++ b/arch/arm/boot/dts/zynq-parallella.dts
 @@ -32,6 +32,31 @@
   bootargs = console=ttyPS0,115200 earlyprintk 
 root=/dev/mmcblk0p2 rootfstype=ext4 rw rootwait;
   linux,stdout-path = /amba/serial@e0001000;
   };
 +
 + fpga {
Do you really want FPGA components in this DT?
If somebody tries booting with this DT without programming with a
corresponding bitstream, the whole system might hang.
Just something to consider.

Sören

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

Re: [PATCH v2 04/11] ARM: dts: zynq: Update deprecated xuartps clock names

[Sören Brinkmann]

On Fri, 2014-07-25 at 01:13AM +0200, Andreas Färber wrote:
 Am 25.07.2014 01:09, schrieb Sören Brinkmann:
  On Fri, 2014-07-25 at 01:00AM +0200, Andreas Färber wrote:
  Avoids deprecation warning messages.
  
  A patch that updates those names is already in armsoc.
 
 Which tree/branch should I base this series on then? It seemed the
 Xilinx branches were all heavily outdated some days ago, so this is
 against vanilla v3.16-rc6.

That is something for the maintainers to answer. But in general, you're
adding a lot of stuff that is not yet upstream, AFAIK. Some is still
under review I think (like QSPI), others are just staged for the next
merge window and parts are still in flight. Michal hopefully has a
better overview.
This particular patch can simply be omitted I think.

Sören

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

Re: [PATCH v2 06/11] ARM: dts: zynq: Add DMAC for Parallella

[Sören Brinkmann]

On Fri, 2014-07-25 at 01:00AM +0200, Andreas Färber wrote:
 Signed-off-by: Andreas Färber afaer...@suse.de
 ---
  v2: New
  
  arch/arm/boot/dts/zynq-7000.dtsi  | 17 +
  arch/arm/boot/dts/zynq-parallella.dts |  4 
  2 files changed, 21 insertions(+)
 
 diff --git a/arch/arm/boot/dts/zynq-7000.dtsi 
 b/arch/arm/boot/dts/zynq-7000.dtsi
 index eed3df0..1a70277 100644
 --- a/arch/arm/boot/dts/zynq-7000.dtsi
 +++ b/arch/arm/boot/dts/zynq-7000.dtsi
 @@ -223,6 +223,23 @@
   };
   };
  
 + dmac_s: dmac@f8003000 {
 + compatible = arm,pl330, arm,primecell;
 + reg = 0xf8003000 0x1000;
 + status = disabled;
I think for this IP we can omit the 'status' property since it is always
enabled. I don't see a reason to override it in each board DT.

 + interrupt-parent = intc;
 + interrupts = 0 13 4,
 +  0 14 4, 0 15 4,
 +  0 16 4, 0 17 4,
 +  0 40 4, 0 41 4,
 +  0 42 4, 0 43 4;
 + #dma-cells = 1;
 + #dma-channels = 8;
 + #dma-requests = 4;
 + clocks = clkc 27;
 + clock-names = apb_pclk;
 + };
 +

Acked-by: Soren Brinkmann soren.brinkm...@xilinx.com

Sören

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

Re: [PATCH] ARM: zynq: DT: Add CAN node

[Sören Brinkmann]

On Wed, 2014-07-23 at 03:05PM +0200, Michal Simek wrote:
 Add node describing Zynq's CAN controller.
 
 Signed-off-by: Michal Simek michal.si...@xilinx.com
 ---
 
  arch/arm/boot/dts/zynq-7000.dtsi | 13 -
  1 file changed, 12 insertions(+), 1 deletion(-)
 
 diff --git a/arch/arm/boot/dts/zynq-7000.dtsi 
 b/arch/arm/boot/dts/zynq-7000.dtsi
 index 366ca6434f54..2287d9b4ed1a 100644
 --- a/arch/arm/boot/dts/zynq-7000.dtsi
 +++ b/arch/arm/boot/dts/zynq-7000.dtsi
 @@ -71,7 +71,18 @@
   interrupts = 0 7 4;
   interrupt-parent = intc;
   clocks = clkc 12;
 - };
 + };
 +
 + can0: can@e0008000 {
 + compatible = xlnx,zynq-can-1.0;
 + clocks = clkc 19, clkc 36;
 + clock-names = can_clk, pclk;
 + reg = 0xe0008000 0x1000;
 + interrupts = 0 28 4;
 + interrupt-parent = intc;
 + tx-fifo-depth = 0x40;
 + rx-fifo-depth = 0x40;
 + };

What about the second CAN core? You also probably want to add
'status = disabled' in the dtsi.

Sören

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

Re: [PATCH v2 1/2] gpio: Add driver for Zynq GPIO controller

[Sören Brinkmann]

Hi Linus,

On Tue, 2014-07-08 at 11:34AM +0200, Linus Walleij wrote:
 On Mon, Jul 7, 2014 at 6:08 PM, Sören Brinkmann
 soren.brinkm...@xilinx.com wrote:
 
  So: what is the usecase for these GPIOs?
 
  Yea, in this case it was a button. I have to look at these drivers. It's
  very likely that they cover what I want. But this case is trivial. I
  really don't do anything but enabling the IRQ by writing to the edge
  attribute and press the push-button connected to that GPIO line.
 
 In case of a system using device tree it is very trivial to add a gpio
 key binding. After compiling in the gpio keys driver this small
 snipper type is all that is really needed in most cases:
 
 /* User key mapped in as escape */
 gpio-keys {
 compatible = gpio-keys;
 user-button {
 label = user_button;
 gpios = gpio0 3 0x1;
 linux,code = 1; /* KEY_ESC */
 gpio-key,wakeup;
 };
 };

Thanks for the example. I think for my cases these drivers are exactly
the right thing.

 
 
  But as a general note: I think we have quite some customers trying to do
  GPIO in userspace.
 
 For what? I mean the use cases. Usually it is a bad idea, and
 as shown above, just using the right existing device driver with
 device tree is much easier, also for an end user, given they know
 how to alter DTs and compile in kernel modules.
 
  With Zynq's FPGA portion, a lot of things come down
  to make signals accessible in Linux and a lot of people do not want or
  need a full blown kernel driver and use GPIO. The request for 'how to
  handle GPIO IRQs in userspace' is pretty common. Often this gets passed
  on to UIO though.
 
 The short answer is don't handle GPIO IRQs in userspace.
 
 Userspace drivers is generally a bad idea and should not be written.
 The kernel is intended to handle hardware.
 
 The above is doubly true for anything involving IRQs. Just think
 of what IRQs are. They are one of the reasons why we have a
 kernel and not just write all software on a system from scratch
 ourselves.

I fully agree and you don't have to convince me. But to a lot of our
customers that are used to use FPGAs, SOCs and Linux are pretty new. You
see a lot of scary stuff. Accessing /dev/mem seems to be a lot of
people's big hammer solution for everything. Then I always perceive it
as great progress if things like the sysfs interface are used instead.

Thanks,
Sören

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

Re: [PATCH v2 1/2] gpio: Add driver for Zynq GPIO controller

[Sören Brinkmann]

Hi Linus,

On Mon, 2014-07-07 at 04:51PM +0200, Linus Walleij wrote:
 On Wed, Jun 18, 2014 at 5:36 PM, Sören Brinkmann
 soren.brinkm...@xilinx.com wrote:
 
  I did some of the changes for this v2 and a few things are not clear to
  me.
 
  The first is, how is userspace supposed to find the correct offset for a
  GPIO pin.
 
 The sysfs interface to GPIO is *NOT* *GOOD* this is universally
 agreed upon.
 
 This needs someone to step in and provide a replacement, my preferred
 mechanism would be a /dev/gpiochip0/... hierarchy using char devices.
 
  E.g. let's say GPIO 10 of this SOC-internal GPIO controller is
  something I want to control. So, I'd export GPIO (chip-base + 10). But
  this chip-base seems pretty variable. v1 of this patch had it hardcoded
  to 0, which resulted in a predictable offset, but apparently was utterly
  wrong. Now I see an offset of 138 for this chip when using my config.
  And when I use multi_v7_defconfig the offset is somewhere in the 800s,
  IIRC. The best I found was the 'label' in the gpiochip's sysfs entry,
  but documentation says that is not necessarily unique, and parsing labes
  seems sub-optimal too.
 
 You see. It is badly designed and needs to be rewritten.
 
 It was merged into the kernel at a time when the GPIO subsystem
 was unmaintained, sadly.
 
  The second issue is a stack trace (below) I see when triggering
  interrupts (e.g. echo rising  edge; and then pushing the connected
  button). Looking at the stack trace, I don't see an obvious error (I
  think I pretty much copied the IRQ registration from the gpio-pl061.c
  driver you mentioned). Is this an issue in this driver or the core code?
 
 Probably.
 
 Using GPIOs for IRQs in userspace is an even worse idea than using
 GPIOs from userspace in general :-D
 
 But before you add any hairy code in userspace, please have a look
 at Documentation/gpio/sysfs.txt:
 
 Note that standard kernel drivers exist for common LEDs and Buttons
  GPIO tasks:  leds-gpio and gpio_keys, respectively. Use those
  instead of talking directly to the GPIOs; they integrate with kernel
  frameworks better than your userspace code could.
 
 So: what is the usecase for these GPIOs?

Yea, in this case it was a button. I have to look at these drivers. It's
very likely that they cover what I want. But this case is trivial. I
really don't do anything but enabling the IRQ by writing to the edge
attribute and press the push-button connected to that GPIO line.

But as a general note: I think we have quite some customers trying to do
GPIO in userspace. With Zynq's FPGA portion, a lot of things come down
to make signals accessible in Linux and a lot of people do not want or
need a full blown kernel driver and use GPIO. The request for 'how to
handle GPIO IRQs in userspace' is pretty common. Often this gets passed
on to UIO though.

Thanks for the answers.

Sören

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

Re: [PATCH v2 2/2] devicetree: Add Zynq GPIO devicetree bindings documentation

[Sören Brinkmann]

On Mon, 2014-07-07 at 04:53PM +0200, Linus Walleij wrote:
 On Wed, Jun 18, 2014 at 1:39 PM, Harini Katakam hari...@xilinx.com wrote:
 
  From: Harini Katakam harini.kata...@xilinx.com
 
  Add gpio-zynq bindings documentation.
 
  Signed-off-by: Harini Katakam hari...@xilinx.com
  Signed-off-by: Soren Brinkmann soren.brinkm...@xilinx.com
  ---
 
  v2 changes:
  Improve description.
 (...)
  +- #gpio-cells  : Should be two. First cell is used to mention
  + pin number.
 
 Don't call this pin number, call it GPIO line number.
It's also not mentioning the second cell at all. I'll take the part from
gpio-msm to document this property and replace 'pin' with 'GPIO line'.

Sören

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

Re: [PATCH v2 1/2] gpio: Add driver for Zynq GPIO controller

[Sören Brinkmann]

On Mon, 2014-07-07 at 04:45PM +0200, Linus Walleij wrote:
 On Wed, Jun 18, 2014 at 1:39 PM, Harini Katakam hari...@xilinx.com wrote:
 
  From: Harini Katakam harini.kata...@xilinx.com
 
  Add support for GPIO controller used by Xilinx Zynq.
 
  Signed-off-by: Harini Katakam hari...@xilinx.com
  Signed-off-by: Soren Brinkmann soren.brinkm...@xilinx.com
  ---
 
  v2 changes:
   - convert to pm_runtime_force_(suspend|resume)
   - add pm_runtime_set_active in probe()
   - also (un)prepare clocks when they are dis-/enabled
   - add some missing calls to pm_runtime_get()
   - use pm_runtime_put() instead of sync variant
   - remove gpio chip in driver remove()
   - remove redundant type casts
   - directly use IO helpers
   - use BIT macro to set/clear bits
   - migrate to GPIOLIB_IRQCHIP
 
 This is a great improvement! Only small stuff remains.
 
  +#include linux/bitops.h
  +#include linux/clk.h
  +#include linux/gpio.h
 
 This should be:
 #include linux/gpio/driver.h
 
 If that doesn't work ... why?
Works just fine.

 
  +/**
  + * struct zynq_gpio - gpio device private data structure
  + * @chip:  instance of the gpio_chip
  + * @base_addr: base address of the GPIO device
  + * @irq:   irq associated with the controller
  + * @clk:   clock resource for this controller
  + */
  +struct zynq_gpio {
  +   struct gpio_chip chip;
  +   void __iomem *base_addr;
  +   int irq;
 
 Why is irq kept around in this struct? It looks like it could just
 be a local variable in probe()?
You're right.

I think that were both part of the legacy in this driver.

Sören

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

Re: [PATCH v2 1/2] gpio: Add driver for Zynq GPIO controller

[Sören Brinkmann]

Hi Linus,

I did some of the changes for this v2 and a few things are not clear to
me.

The first is, how is userspace supposed to find the correct offset for a
GPIO pin. E.g. let's say GPIO 10 of this SOC-internal GPIO controller is
something I want to control. So, I'd export GPIO (chip-base + 10). But
this chip-base seems pretty variable. v1 of this patch had it hardcoded
to 0, which resulted in a predictable offset, but apparently was utterly
wrong. Now I see an offset of 138 for this chip when using my config.
And when I use multi_v7_defconfig the offset is somewhere in the 800s,
IIRC. The best I found was the 'label' in the gpiochip's sysfs entry,
but documentation says that is not necessarily unique, and parsing labes
seems sub-optimal too.

The second issue is a stack trace (below) I see when triggering
interrupts (e.g. echo rising  edge; and then pushing the connected
button). Looking at the stack trace, I don't see an obvious error (I
think I pretty much copied the IRQ registration from the gpio-pl061.c
driver you mentioned). Is this an issue in this driver or the core code?
This happens on Linus' latest tip. Despite all this chatter the system
still works and doesn't not seem to lock up.

Here the stack trace:
# echo rising  edge
  BUG: sleeping function called from invalid context at 
kernel/locking/mutex.c:586
  in_atomic(): 1, irqs_disabled(): 128, pid: 0, name: swapper/0
  no locks held by swapper/0/0.
  irq event stamp: 55488
  hardirqs last  enabled at (55485): [c03bf10c] cpuidle_enter_state+0x60/0xec
  hardirqs last disabled at (55486): [c0014374] __irq_svc+0x34/0x78
  softirqs last  enabled at (55488): [c002fc40] _local_bh_enable+0x58/0x64
  softirqs last disabled at (55487): [c0030534] irq_enter+0x48/0x88
  CPU: 0 PID: 0 Comm: swapper/0 Not tainted 
3.16.0-rc1-xilinx-00021-gf5ddad957172 #88
  [c0017840] (unwind_backtrace) from [c0013770] (show_stack+0x20/0x24)
  [c0013770] (show_stack) from [c04d94a0] (dump_stack+0x8c/0xd0)
  [c04d94a0] (dump_stack) from [c005d5e4] (__might_sleep+0x1ac/0x1e4)
  [c005d5e4] (__might_sleep) from [c04dc19c] (mutex_lock_nested+0x40/0x46c)
  [c04dc19c] (mutex_lock_nested) from [c019df34] (kernfs_notify+0xac/0x148)
  [c019df34] (kernfs_notify) from [c02d89e0] (gpio_sysfs_irq+0x1c/0x24)
  [c02d89e0] (gpio_sysfs_irq) from [c008588c] 
(handle_irq_event_percpu+0xa8/0x3c0)
  [c008588c] (handle_irq_event_percpu) from [c0085bf0] 
(handle_irq_event+0x4c/0x6c)
  [c0085bf0] (handle_irq_event) from [c0088a28] 
(handle_simple_irq+0xac/0xbc)
  [c0088a28] (handle_simple_irq) from [c0084fec] 
(generic_handle_irq+0x30/0x40)
  [c0084fec] (generic_handle_irq) from [c02de3fc] 
(zynq_gpio_irqhandler+0xe8/0x130)
  [c02de3fc] (zynq_gpio_irqhandler) from [c0084fec] 
(generic_handle_irq+0x30/0x40)
  [c0084fec] (generic_handle_irq) from [c000fd24] (handle_IRQ+0x78/0xa0)
  [c000fd24] (handle_IRQ) from [c000868c] (gic_handle_irq+0x4c/0x70)
  [c000868c] (gic_handle_irq) from [c0014384] (__irq_svc+0x44/0x78)
  Exception stack(0xc0755ec8 to 0xc0755f10)
  5ec0:   0001 0004  c075fb70 0001 edfc9310
  5ee0: 8ed05707 001d 9e7f7ffa 001d c0752308 c0755f44 c0755ee0 c0755f10
  5f00: c0077340 c03bf110 200d0013 
  [c0014384] (__irq_svc) from [c03bf110] (cpuidle_enter_state+0x64/0xec)
  [c03bf110] (cpuidle_enter_state) from [c03bf2a0] (cpuidle_enter+0x24/0x28)
  [c03bf2a0] (cpuidle_enter) from [c0071d94] (cpu_idle_loop+0x2e0/0x6fc)
  [c0071d94] (cpu_idle_loop) from [c00721cc] (cpupri_find+0x0/0x108)
  
  =
  [ INFO: inconsistent lock state ]
  3.16.0-rc1-xilinx-00021-gf5ddad957172 #88 Not tainted
  -
  inconsistent {HARDIRQ-ON-W} - {IN-HARDIRQ-W} usage.
  swapper/0/0 [HC1[1]:SC0[0]:HE0:SE1] takes:
   (kernfs_mutex){?.+.+.}, at: [c019df34] kernfs_notify+0xac/0x148
  {HARDIRQ-ON-W} state was registered at:
[c0079a08] lock_acquire+0xfc/0x21c
[c04dc1e0] mutex_lock_nested+0x84/0x46c
[c019d668] kernfs_activate+0x2c/0xf8
[c019d9b8] kernfs_create_root+0xbc/0xe0
[c070edac] sysfs_init+0x20/0x5c
[c070cef4] mnt_init+0x108/0x258
[c070caa8] vfs_caches_init+0x9c/0x110
[c06f5c3c] start_kernel+0x364/0x3fc
[8074] 0x8074
  irq event stamp: 55488
  hardirqs last  enabled at (55485): [c03bf10c] cpuidle_enter_state+0x60/0xec
  hardirqs last disabled at (55486): [c0014374] __irq_svc+0x34/0x78
  softirqs last  enabled at (55488): [c002fc40] _local_bh_enable+0x58/0x64
  softirqs last disabled at (55487): [c0030534] irq_enter+0x48/0x88
  
  other info that might help us debug this:
   Possible unsafe locking scenario:
  
 CPU0
 
lock(kernfs_mutex);
Interrupt
  lock(kernfs_mutex);
  
   *** DEADLOCK ***
  
  no locks held by swapper/0/0.
  
  stack backtrace:
  CPU: 0 PID: 0 Comm: swapper/0 Not tainted 
3.16.0-rc1-xilinx-00021-gf5ddad957172 #88
  [c0017840] (unwind_backtrace) from [c0013770] (show_stack+0x20/0x24)
  [c0013770] (show_stack) 

Re: Compatible string best practices

[Sören Brinkmann]

On Fri, 2014-05-23 at 05:03PM +0100, Mark Rutland wrote:
 On Fri, May 23, 2014 at 04:03:06PM +0100, Sören Brinkmann wrote:
  Hi,
 
 Hi,
 
  I have a question regarding how a compatible string should be created.
  The simple case is obvious:
   IP-vendor,IP-type/name-IP-revision
  
  But what is the recommended string for a SOC specific implementation of
  that IP? Let's say for
   SOC-vendor,SOC-name-SOC-revision
  how would I assemble a compatible string to identify the SOC-specific
  implementation of IP?
  I think my biggest confusion is which vendor string to use in that case.
  But in general I'm curious whether some best practice exists for these
  cases.
  
  I think what I've seen most is:
   SOC-vendor,SOC-name-IP-type
  
  But one could probably argue whether to rather use IP-vendor, appending
  all the revision strings, ...
 
 I would expect that the vendor prefix would be the last entity along the
 chain which altered the IP in some way. Say foo sells device to bar, who
 do nothing other than sell it to baz who do some hacks to integrate it.
 For that I'd expect something like:
 
 compatible = baz,soc-device, foo,device;
 
 I'm not sure with revisions (they're not always that useful), but I'd
 expect that to be kept next to the element it represent the revision
 for:
 
 baz,soc-vX-device-vY, bar,soc-device-vY, foo,device-vY,
 foo,device-vY;

Thanks, Mark!

Sören

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

Compatible string best practices

[Sören Brinkmann]

Hi,

I have a question regarding how a compatible string should be created.
The simple case is obvious:
 IP-vendor,IP-type/name-IP-revision

But what is the recommended string for a SOC specific implementation of
that IP? Let's say for
 SOC-vendor,SOC-name-SOC-revision
how would I assemble a compatible string to identify the SOC-specific
implementation of IP?
I think my biggest confusion is which vendor string to use in that case.
But in general I'm curious whether some best practice exists for these
cases.

I think what I've seen most is:
 SOC-vendor,SOC-name-IP-type

But one could probably argue whether to rather use IP-vendor, appending
all the revision strings, ...

Thanks,
Sören

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

Re: [RFC] Migrate to Hz resolution for OPP binding

[Sören Brinkmann]

Hi Rob,

On Thu, 2014-05-22 at 08:27AM -0500, Rob Herring wrote:
 On Tue, May 20, 2014 at 5:30 PM, Sören Brinkmann
 soren.brinkm...@xilinx.com wrote:
  Hi,
 
  I guess this is just to evaluate how big the lynch mob will be. Anyway:
  Triggered by this discussion https://lkml.org/lkml/2014/5/15/46, I
  looked a little into what it would take to migrate everybody to Hz
  frequency resolutions to avoid all the conversions between cpufreq, CCF,
  OPPs, etc.
  Turns out, OPPs are already stored in Hz resolution in the kernel, but the 
  DT
  bindings use kHz resolution to specify them in DT. So, code-wise there
  is just a removal of a multiplication in the OPP parser (see below), but
  then there are the DT bindings...
 
 Add this to the list of issues with the current OPP binding and define
 a new binding, not a band-aid.
 
  As plan B, I was thinking to add the property 'operating-points-hz'...
 
  Any better ideas?
 
 Fix clk_round_rate to specify how you want to round. There are
 usecases for both rounding up and down. Don't you have the same
 rounding problem with hz? Assume the person filling in OPP values
 knows nothing about the kernel and you could have values rounded both
 ways.

Yes the issue would not be resolved by this alone. But currently the CCF
and OPPs in the kernel use Hz. And if you look at the interaction
between those frameworks with cpufreq, there are plenty of
multiplications/divisions by 1000 to convert between those, with even a
greater loss of accuracy. So, I think agreeing on using Hz as smallest
reasonable frequency resolution across all these frameworks would be a
first step into the right direction.

Thanks,
Sören
--
To unsubscribe from this list: send the line unsubscribe devicetree in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 2/6] dt-bindings: add mtk-timer bindings

[Sören Brinkmann]

Hi Matthias,

On Wed, 2014-05-21 at 06:26PM +0200, Matthias Brugger wrote:
 Add binding documentation for the General Porpose Timer driver of

Typo: Purpose

 the Mediatek SoCs.
 
 Signed-off-by: Matthias Brugger matthias@gmail.com
 ---
  .../devicetree/bindings/timer/mediatek,mtk-timer.txt   | 18 
 ++
  1 file changed, 18 insertions(+)
  create mode 100644 
 Documentation/devicetree/bindings/timer/mediatek,mtk-timer.txt
 
 diff --git a/Documentation/devicetree/bindings/timer/mediatek,mtk-timer.txt 
 b/Documentation/devicetree/bindings/timer/mediatek,mtk-timer.txt
 new file mode 100644
 index 000..7d909f7
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/timer/mediatek,mtk-timer.txt
 @@ -0,0 +1,18 @@
 +Mediatek MT6577, MT6572 and MT6589 Timers
 +---
 +
 +Required properties:
 +- compatible: Should be mediatek,mtk6577-timer
 +- reg: Should contain location and length for timers register.
 +- clocks: Clocks driving the timer hardware. This list shoud include two
 + clocks. The order is system clock and as second clock the RTC clock.
 +
 +Examples:
 +
 + timer {

The node name should be timer@0x10008000

 + compatible = mediatek,mtk6577-timer;
 + reg = 0x10008000 0x80;
 + interrupts = GIC_SPI 113 IRQ_TYPE_LEVEL_LOW;
 + clocks = system_clk, rtc_clk;
 + clock-names = system-clk, rtc-clk;

I'm still not convinced that the timer IP calls its clock inputs this
way, but well.

Other than that ACK.

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

Re: [PATCH v5 2/6] dt-bindings: add mtk-timer bindings

[Sören Brinkmann]

On Wed, 2014-05-21 at 06:54PM +0200, Heiko Stübner wrote:
 Am Mittwoch, 21. Mai 2014, 09:34:10 schrieb Sören Brinkmann:
  Hi Matthias,
  
  On Wed, 2014-05-21 at 06:26PM +0200, Matthias Brugger wrote:
   Add binding documentation for the General Porpose Timer driver of
  
  Typo: Purpose
  
   the Mediatek SoCs.
   
   Signed-off-by: Matthias Brugger matthias@gmail.com
   ---
   
.../devicetree/bindings/timer/mediatek,mtk-timer.txt   | 18
++ 1 file changed, 18 insertions(+)
create mode 100644
Documentation/devicetree/bindings/timer/mediatek,mtk-timer.txt 
   diff --git
   a/Documentation/devicetree/bindings/timer/mediatek,mtk-timer.txt
   b/Documentation/devicetree/bindings/timer/mediatek,mtk-timer.txt new file
   mode 100644
   index 000..7d909f7
   --- /dev/null
   +++ b/Documentation/devicetree/bindings/timer/mediatek,mtk-timer.txt
   @@ -0,0 +1,18 @@
   +Mediatek MT6577, MT6572 and MT6589 Timers
   +---
   +
   +Required properties:
   +- compatible: Should be mediatek,mtk6577-timer
   +- reg: Should contain location and length for timers register.
   +- clocks: Clocks driving the timer hardware. This list shoud include two
   + clocks. The order is system clock and as second clock the RTC clock.
   +
   +Examples:
   +
   + timer {
  
  The node name should be timer@0x10008000
 
 I think the more commonly used variant is
 
   timer@10008000
 
 without the 0x. While some dts files really use @0x they are in the minority.

You're right.

 
 
  
   + compatible = mediatek,mtk6577-timer;
   + reg = 0x10008000 0x80;
   + interrupts = GIC_SPI 113 IRQ_TYPE_LEVEL_LOW;
   + clocks = system_clk, rtc_clk;
   + clock-names = system-clk, rtc-clk;
  
  I'm still not convinced that the timer IP calls its clock inputs this
  way, but well.
 
 Maybe this might convince you ;-)
 
 The GPT includes 5 32-bit timers and one 64-bit timer. Each timer has 4 
 operation modes, which are ONE-SHOT,  REPEAT,  KEEP-GO and  FREERUN, and can 
 operate on one of the 2 clock sources, RTC clock (32.768kHz) and system clock 
 (13MHz).

Is this copied from the timer data sheet or the SOC documentation?

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

Re: [PATCH v5 2/6] dt-bindings: add mtk-timer bindings

[Sören Brinkmann]

On Wed, 2014-05-21 at 07:18PM +0200, Heiko Stübner wrote:
 Am Mittwoch, 21. Mai 2014, 09:53:57 schrieb Sören Brinkmann:
  On Wed, 2014-05-21 at 06:54PM +0200, Heiko Stübner wrote:
   Am Mittwoch, 21. Mai 2014, 09:34:10 schrieb Sören Brinkmann:
Hi Matthias,

On Wed, 2014-05-21 at 06:26PM +0200, Matthias Brugger wrote:
 + compatible = mediatek,mtk6577-timer;
 + reg = 0x10008000 0x80;
 + interrupts = GIC_SPI 113 IRQ_TYPE_LEVEL_LOW;
 + clocks = system_clk, rtc_clk;
 + clock-names = system-clk, rtc-clk;

I'm still not convinced that the timer IP calls its clock inputs this
way, but well.
   
   Maybe this might convince you ;-)
   
   The GPT includes 5 32-bit timers and one 64-bit timer. Each timer has 4
   operation modes, which are ONE-SHOT,  REPEAT,  KEEP-GO and  FREERUN, and
   can operate on one of the 2 clock sources, RTC clock (32.768kHz) and
   system clock (13MHz).
  
  Is this copied from the timer data sheet or the SOC documentation?
 
 That is from the processor datasheet containing the timer documentation.
 I don't think it's customary for soc vendors to provide additional individual 
 documentation for self-developed IPs contained in a SoC.

May be, but that is what might create the problem I'm talking about. The
next generation SOC using the same IP may use different clocks to
drive it. Now you're stuck with RTC and system clock as names, but they
don't apply to the latest integration anymore. That's why I advocate
using the IP's naming for its clock signals as opposed to the SOC's
clock names.
So, my concern is still standing. But I see, that this might not be
resolvable, in case such information is not available. Nevertheless,
I see the currently used names as implementation details of the SOC
and not a property of the here described timer IP. But might be that
this is as good as it gets.

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