Re: SoCFPGA ethernet broken

[Dinh Nguyen]

On 12/03/2015 03:23 PM, David Daney wrote:
> On 12/03/2015 12:48 PM, Pavel Machek wrote:
>> On Thu 2015-10-15 13:25:59, Florian Fainelli wrote:
>>> On 15/10/15 12:59, Dinh Nguyen wrote:
>>>> On 10/15/2015 03:03 PM, Florian Fainelli wrote:
>>>>> On 15/10/15 12:09, Dinh Nguyen wrote:
>>>>>> Hi,
>>>>>>
>>>>>> commit "8b63ec1837fa phylib: Make PHYs children of their MDIO bus,
>>>>>> not
>>>>>> the bus' parent." seems to have broken ethernet support for the
>>>>>> SoCFPGA
>>>>>> platform which is using the stmmac ethernet driver.
>>>>>
>>>>> It is not clear to me how this relates to what you are seeing yet.
>>>>>
>>>>>>
>>>>>> It appears that during DHCP, it cannot get an IP address. This only
>>>>>> happens if ethernet was not used by the bootloader to tftp an kernel
>>>>>> image. If I use the bootloader to tftp an image then ethernet is
>>>>>> working
>>>>>> fine. So I think the PHY is not getting enabled properly.
>>>>>>
>>>>>> If I revert this patch, then ethernet is back to working on the
>>>>>> platform.
>>>>>
>>>>> Is the Device Tree source for this platform available somewhere to
>>>>> look at?
>>>>>
>>>>
>>>> Yes, I'm using the DTS that is in the mainline:
>>>>
>>>> arch/arm/boot/dts/socfpga.dtsi
>>>> arch/arm/boot/dts/socfpga_cyclone5.dtsi
>>>> arch/arm/boot/dts/socfpga_cyclone5_socdk.dts
>>>
>>> There are no PHY devices in any of these DTS files, instead there is the
>>> non-standard "phy-addr" property which is set to 0x supposedly
>>> to indicate that the MDIO bus should be scanned. This is likely part of
>>> your problem. The stmmac driver seems to be looking for "snps,phy-addr"
>>> and not "phy-addr", so I am not even clear how this is supposed to work,
>>> and the driver mentions this custom property is deprecated anyway.
>>>
>>> The core problem is in
>>> drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c::stmmac_mdio_register
>>> which manually detects the PHY, that is mostly fine, except that it does
>>> not really seem to work here for a reason that is still unclear to me.
>>>
>>> Your Ethernet PHYs need to be declared in Device Tree, see
>>> Documentation/devicetree/bindings/net/phy.txt
>>
>> While updating DTS might be good idea, I don't think you can simply
>> blame this on DTS. If it worked before the change, it is supposed to
>> work after the change, otherwise we call that change a "regression"
>> and revert the change.
> 
> FWIW: My initial patch to address the failure worked with the original DTB.
> 

Can I ask what patch are you referring to? I was sidetracked for a while
on this issue, but I still see it failing as of v4.4-rc3. I'll try to
get back to debugging this.

Dinh

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: Denali NAND driver on SocKit

[Dinh Nguyen]

CC: Graham Moore

Graham should know more about this.

Dinh

On 11/30/2015 12:31 PM, Pavel Machek wrote:
> Hi!
> 
> I'm trying to get NAND to work on socdk board... but it looks like
> neccessary device tree bindings are not there. And when I do add them,
> they don't seem to work.
> 
> Does it work for you? Is there patch I should apply?
> 
> Thanks,
>   Pavel
> 
> 
> diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi
> index bd2937f..ec61b29 100644
> --- a/arch/arm/boot/dts/socfpga.dtsi
> +++ b/arch/arm/boot/dts/socfpga.dtsi
> @@ -582,6 +582,46 @@
>   status = "disabled";
>   };
>  
> + nand: nand@ff90 {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + compatible = "denali,denali-nand-dt";
> + reg = <0xff90 0x10>, <0xffb8 0x1>;
> + reg-names = "nand_data", "denali_reg";
> + interrupts = <0 144 4>;
> + dma-mask = <0x>;
> + clocks = <_clk>;
> + have-hw-ecc-fixup;
> + status = "disabled";
> +
> + partition@nand-boot {
> + /* 8MB for raw data. */
> + label = "NAND Flash Boot Area 8MB";
> + reg = <0x0 0x80>;
> + };
> + partition@nand-rootfs {
> + /* 128MB jffs2 root filesystem. */
> + label = "NAND Flash jffs2 Root Filesystem 
> 128MB";
> + reg = <0x80 0x800>;
> + };
> + partition@nand-128 {
> + label = "NAND Flash 128 MB";
> + reg = <0x880 0x800>;
> + };
> + partition@nand-64 {
> + label = "NAND Flash 64 MB";
> + reg = <0x1080 0x400>;
> + };
> + partition@nand-32 {
> + label = "NAND Flash 32 MB";
> + reg = <0x1480 0x200>;
> + };
> + partition@nand-16 {
> + label = "NAND Flash 16 MB";
> + reg = <0x1680 0x100>;
> + };
> + };
> +
>   gpio0: gpio@ff708000 {
>   #address-cells = <1>;
>   #size-cells = <0>;
> diff --git a/arch/arm/boot/dts/socfpga_cyclone5_socdk.dts 
> b/arch/arm/boot/dts/socfpga_cyclone5_socdk.dts
> index d4d0a28..b56c8f9 100644
> --- a/arch/arm/boot/dts/socfpga_cyclone5_socdk.dts
> +++ b/arch/arm/boot/dts/socfpga_cyclone5_socdk.dts
> @@ -89,3 +89,7 @@
>   {
>   status = "okay";
>  };
> +
> + {
> +status = "okay";
> +};
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: Denali NAND driver on SocKit

[Dinh Nguyen]

CC: Graham Moore

Graham should know more about this.

Dinh

On 11/30/2015 12:31 PM, Pavel Machek wrote:
> Hi!
> 
> I'm trying to get NAND to work on socdk board... but it looks like
> neccessary device tree bindings are not there. And when I do add them,
> they don't seem to work.
> 
> Does it work for you? Is there patch I should apply?
> 
> Thanks,
>   Pavel
> 
> 
> diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi
> index bd2937f..ec61b29 100644
> --- a/arch/arm/boot/dts/socfpga.dtsi
> +++ b/arch/arm/boot/dts/socfpga.dtsi
> @@ -582,6 +582,46 @@
>   status = "disabled";
>   };
>  
> + nand: nand@ff90 {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + compatible = "denali,denali-nand-dt";
> + reg = <0xff90 0x10>, <0xffb8 0x1>;
> + reg-names = "nand_data", "denali_reg";
> + interrupts = <0 144 4>;
> + dma-mask = <0x>;
> + clocks = <_clk>;
> + have-hw-ecc-fixup;
> + status = "disabled";
> +
> + partition@nand-boot {
> + /* 8MB for raw data. */
> + label = "NAND Flash Boot Area 8MB";
> + reg = <0x0 0x80>;
> + };
> + partition@nand-rootfs {
> + /* 128MB jffs2 root filesystem. */
> + label = "NAND Flash jffs2 Root Filesystem 
> 128MB";
> + reg = <0x80 0x800>;
> + };
> + partition@nand-128 {
> + label = "NAND Flash 128 MB";
> + reg = <0x880 0x800>;
> + };
> + partition@nand-64 {
> + label = "NAND Flash 64 MB";
> + reg = <0x1080 0x400>;
> + };
> + partition@nand-32 {
> + label = "NAND Flash 32 MB";
> + reg = <0x1480 0x200>;
> + };
> + partition@nand-16 {
> + label = "NAND Flash 16 MB";
> + reg = <0x1680 0x100>;
> + };
> + };
> +
>   gpio0: gpio@ff708000 {
>   #address-cells = <1>;
>   #size-cells = <0>;
> diff --git a/arch/arm/boot/dts/socfpga_cyclone5_socdk.dts 
> b/arch/arm/boot/dts/socfpga_cyclone5_socdk.dts
> index d4d0a28..b56c8f9 100644
> --- a/arch/arm/boot/dts/socfpga_cyclone5_socdk.dts
> +++ b/arch/arm/boot/dts/socfpga_cyclone5_socdk.dts
> @@ -89,3 +89,7 @@
>   {
>   status = "okay";
>  };
> +
> + {
> +status = "okay";
> +};
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: SoCFPGA ethernet broken

[Dinh Nguyen]

+CC Giuseppe Cavallaro
+CC STi and Rockchip Maintainers

This is approaching beyond my breadth of knowledge on this subject, so I just
wanted to get some further insight.

On Fri, 16 Oct 2015, Andrew Lunn wrote:

> > > Maybe we need to walk up the hierarchy.
> > > 
> > > Perhaps something like:
> > > 
> > > const struct device *dev_walker;
> > > 
> > > dev_walker = >dev;
> > > do {
> > >of_node = dev_walker->of_node;
> > >dev_walker = dev_walker->parent;
> > > } while (!of_node && dev_walker);
> > > 
> > 
> > The above code seems to have fixed the issue.
> 
> What i don't like about this is that it allows you to put these
> properties in the mdio device node. These are phy properties, not mdio
> properties
>

AFAICT, the stmmac driver is allowing for the phy node to be part of the mdio.
In the function, stmmac_init_phy(), there is a separate check of a standalone
phy_node, and the case where the phy is part of the mdio.

commit "8b63ec1837fa phylib: Make PHYs children of their MDIO bus, not
the bus' parent." is now placing a hard requirement to have a PHY as a
separate node. For now this is only breaking SoCFPGA, but perhaps might
have affected STi41x and RockChip, but maybe haven't been spotted because
perhaps the testing has the bootloader setting up the PHYs?

> If phydev->attached_dev->dev->of_node works, that would be my
> preference.
>

BR,
Dinh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: SoCFPGA ethernet broken

[Dinh Nguyen]

On Mon, 19 Oct 2015, Dinh Nguyen wrote:

+CC Giuseppe Cavallaro
+CC STi and Rockchip Maintainers

This is approaching beyond my breadth of knowledge on this subject, so I just
wanted to get some further insight.

> 
> On Fri, 16 Oct 2015, Andrew Lunn wrote:
> 
> > > > Maybe we need to walk up the hierarchy.
> > > > 
> > > > Perhaps something like:
> > > > 
> > > > const struct device *dev_walker;
> > > > 
> > > > dev_walker = >dev;
> > > > do {
> > > >of_node = dev_walker->of_node;
> > > >dev_walker = dev_walker->parent;
> > > > } while (!of_node && dev_walker);
> > > > 
> > > 
> > > The above code seems to have fixed the issue.
> > 
> > What i don't like about this is that it allows you to put these
> > properties in the mdio device node. These are phy properties, not mdio
> > properties
> >
> 
AFAICT, the stmmac driver is allowing for the phy node to be part of the mdio.
In the function, stmmac_init_phy(), there is a separate check of a standalone
phy_node, and the case where the phy is part of the mdio.

commit "8b63ec1837fa phylib: Make PHYs children of their MDIO bus, not
the bus' parent." is now now placing a hard requirement that the phy must be
in a separate node. For now, this is breaking SoCFPGA, but I think it may also
impact Rockchip and STi?

> > If phydev->attached_dev->dev->of_node works, that would be my
> > preference.
> >

BR,
Dinh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: SoCFPGA ethernet broken

[Dinh Nguyen]

On Mon, 19 Oct 2015, Dinh Nguyen wrote:

+CC Giuseppe Cavallaro
+CC STi and Rockchip Maintainers

This is approaching beyond my breadth of knowledge on this subject, so I just
wanted to get some further insight.

> 
> On Fri, 16 Oct 2015, Andrew Lunn wrote:
> 
> > > > Maybe we need to walk up the hierarchy.
> > > > 
> > > > Perhaps something like:
> > > > 
> > > > const struct device *dev_walker;
> > > > 
> > > > dev_walker = >dev;
> > > > do {
> > > >of_node = dev_walker->of_node;
> > > >dev_walker = dev_walker->parent;
> > > > } while (!of_node && dev_walker);
> > > > 
> > > 
> > > The above code seems to have fixed the issue.
> > 
> > What i don't like about this is that it allows you to put these
> > properties in the mdio device node. These are phy properties, not mdio
> > properties
> >
> 
AFAICT, the stmmac driver is allowing for the phy node to be part of the mdio.
In the function, stmmac_init_phy(), there is a separate check of a standalone
phy_node, and the case where the phy is part of the mdio.

commit "8b63ec1837fa phylib: Make PHYs children of their MDIO bus, not
the bus' parent." is now now placing a hard requirement that the phy must be
in a separate node. For now, this is breaking SoCFPGA, but I think it may also
impact Rockchip and STi?

> > If phydev->attached_dev->dev->of_node works, that would be my
> > preference.
> >

BR,
Dinh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: SoCFPGA ethernet broken

[Dinh Nguyen]

+CC Giuseppe Cavallaro
+CC STi and Rockchip Maintainers

This is approaching beyond my breadth of knowledge on this subject, so I just
wanted to get some further insight.

On Fri, 16 Oct 2015, Andrew Lunn wrote:

> > > Maybe we need to walk up the hierarchy.
> > > 
> > > Perhaps something like:
> > > 
> > > const struct device *dev_walker;
> > > 
> > > dev_walker = >dev;
> > > do {
> > >of_node = dev_walker->of_node;
> > >dev_walker = dev_walker->parent;
> > > } while (!of_node && dev_walker);
> > > 
> > 
> > The above code seems to have fixed the issue.
> 
> What i don't like about this is that it allows you to put these
> properties in the mdio device node. These are phy properties, not mdio
> properties
>

AFAICT, the stmmac driver is allowing for the phy node to be part of the mdio.
In the function, stmmac_init_phy(), there is a separate check of a standalone
phy_node, and the case where the phy is part of the mdio.

commit "8b63ec1837fa phylib: Make PHYs children of their MDIO bus, not
the bus' parent." is now placing a hard requirement to have a PHY as a
separate node. For now this is only breaking SoCFPGA, but perhaps might
have affected STi41x and RockChip, but maybe haven't been spotted because
perhaps the testing has the bootloader setting up the PHYs?

> If phydev->attached_dev->dev->of_node works, that would be my
> preference.
>

BR,
Dinh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: SoCFPGA ethernet broken

[Dinh Nguyen]

On Fri, 16 Oct 2015, David Daney wrote:

> On 10/16/2015 08:56 AM, Andrew Lunn wrote:
> > > So I think I'll move to inspect what Florian had suggested, and that was
> > > to look
> > > at:
> > > drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c::stmmac_mdio_register
> > 
> > I have a suspicion. If you look at the phy driver it does:
> > 
> > static int ksz9021_config_init(struct phy_device *phydev)
> > {
> >  const struct device *dev = >dev;
> >  const struct device_node *of_node = dev->of_node;
> > 
> >  if (!of_node && dev->parent->of_node)
> >  of_node = dev->parent->of_node;
> > 
> 
> Maybe we need to walk up the hierarchy.
> 
> Perhaps something like:
> 
> const struct device *dev_walker;
> 
> dev_walker = >dev;
> do {
>of_node = dev_walker->of_node;
>dev_walker = dev_walker->parent;
> } while (!of_node && dev_walker);
> 

The above code seems to have fixed the issue.

> An alternative would be to assign the bus the same of_node as the bus parent.
> 
> If either approach works, you can add:
>  Acked-by: David Daney 
> 
> to the patch that implements it.
> 
> > 
> > In your case, you don't have a phy node in your device tree, so of_node
> > is NULL. So it looks in the parent device.
> > 
> >  phylib: Make PHYs children of their MDIO bus, not the bus' parent.
> > 
> > changed what the parent is. It is now the mdio device. Before, i
> > suspect it was the MAC. Hence it found your properties in the MAC
> > node.
> > 
> > What i think you might want to do is change this code. Rather than
> > look a dev->parent->of_node; you might want
> > phydev->attached_dev->dev->of_node.
> > 
> > This assumes the phy has been attached to the MAC. I've no idea of the
> > ordering, so maybe it has not been attached yet?
> > 
> > dp83867.c has similar code. However quick grep did not find any
> > mainline users with properties in the MAC node. If that is true, i
> > would suggest removing the code looking in the parent for that phy
> > driver.
> > 
> > Andrew
> > 
> 
> 

BR,
Dinh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: SoCFPGA ethernet broken

[Dinh Nguyen]

On Fri, 16 Oct 2015, Andrew Lunn wrote:

> On Fri, Oct 16, 2015 at 09:38:37AM -0500, Dinh Nguyen wrote:
> > On Fri, 16 Oct 2015, Andrew Lunn wrote:
> > 
> > > > Another debugging point, the SoCFPGA board has a Micrel ksz9021 PHY 
> > > > attached
> > > > to the ethernet port. What I'm seeing is that with 8b63ec1837fa patch, 
> > > > when
> > > > the call to ksz9021_config_init() is made both of_node and 
> > > > dev->parent->of_node
> > > > are NULL, without the patch the dev->parent->of_node is a valid 
> > > > pointer. Thus
> > > > the skew values get programmed to the phy.
> > > 
> > > Ah!
> > > 
> > > You have the phy device tree parameters in the wrong place. These are
> > > phy paramters, so should really be in the phy node. But
> > > socfpga_cyclone5_socdk.dts has them in the MAC node.
> > >
> > 
> > Alright, let me see if I can rework the DTS.
> 
> Well, we are not supposed to break device tree bindings. So we should
> try to make this work again.
>  

Great..thanks for recognizing that point.
 
> > > There is nothing in Documentation/devicetree/bindings/net/micrel.txt
> > > which says you are allowed to place them in the MAC node. Obviously
> > > the code did allow this, which is what has now broken.
> > 
> > I was following Documentation/devicetree/bindings/net/micrel-ksz90x1.txt and
> > in this document I was following the autodetected PHY example. Did I 
> > mis-interpret
> > the example?
> 
> I was looking at the wrong binding documentation. So yes, it is
> documented you can do this.
> 
> But i still think it is wrong. These are phy properties, implemented
> by the phy, so should be in the phy node. So moving them would be
> good. But as i said, we should fix backwards compatibility if
> possible.
>

I moved the phy settings to a separate phy node, but it did not seem to make a
difference at all.

So I think I'll move to inspect what Florian had suggested, and that was to look
at: drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c::stmmac_mdio_register 

BR,
Dinh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: SoCFPGA ethernet broken

[Dinh Nguyen]

On Fri, 16 Oct 2015, Andrew Lunn wrote:

> > Another debugging point, the SoCFPGA board has a Micrel ksz9021 PHY attached
> > to the ethernet port. What I'm seeing is that with 8b63ec1837fa patch, when
> > the call to ksz9021_config_init() is made both of_node and 
> > dev->parent->of_node
> > are NULL, without the patch the dev->parent->of_node is a valid pointer. 
> > Thus
> > the skew values get programmed to the phy.
> 
> Ah!
> 
> You have the phy device tree parameters in the wrong place. These are
> phy paramters, so should really be in the phy node. But
> socfpga_cyclone5_socdk.dts has them in the MAC node.
>

Alright, let me see if I can rework the DTS.
 
> There is nothing in Documentation/devicetree/bindings/net/micrel.txt
> which says you are allowed to place them in the MAC node. Obviously
> the code did allow this, which is what has now broken.
>

I was following Documentation/devicetree/bindings/net/micrel-ksz90x1.txt and
in this document I was following the autodetected PHY example. Did I 
mis-interpret
the example?

BR,
Dinh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: SoCFPGA ethernet broken

[Dinh Nguyen]

On Fri, 16 Oct 2015, David Daney wrote:

> On 10/16/2015 08:56 AM, Andrew Lunn wrote:
> > > So I think I'll move to inspect what Florian had suggested, and that was
> > > to look
> > > at:
> > > drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c::stmmac_mdio_register
> > 
> > I have a suspicion. If you look at the phy driver it does:
> > 
> > static int ksz9021_config_init(struct phy_device *phydev)
> > {
> >  const struct device *dev = >dev;
> >  const struct device_node *of_node = dev->of_node;
> > 
> >  if (!of_node && dev->parent->of_node)
> >  of_node = dev->parent->of_node;
> > 
> 
> Maybe we need to walk up the hierarchy.
> 
> Perhaps something like:
> 
> const struct device *dev_walker;
> 
> dev_walker = >dev;
> do {
>of_node = dev_walker->of_node;
>dev_walker = dev_walker->parent;
> } while (!of_node && dev_walker);
> 

The above code seems to have fixed the issue.

> An alternative would be to assign the bus the same of_node as the bus parent.
> 
> If either approach works, you can add:
>  Acked-by: David Daney 
> 
> to the patch that implements it.
> 
> > 
> > In your case, you don't have a phy node in your device tree, so of_node
> > is NULL. So it looks in the parent device.
> > 
> >  phylib: Make PHYs children of their MDIO bus, not the bus' parent.
> > 
> > changed what the parent is. It is now the mdio device. Before, i
> > suspect it was the MAC. Hence it found your properties in the MAC
> > node.
> > 
> > What i think you might want to do is change this code. Rather than
> > look a dev->parent->of_node; you might want
> > phydev->attached_dev->dev->of_node.
> > 
> > This assumes the phy has been attached to the MAC. I've no idea of the
> > ordering, so maybe it has not been attached yet?
> > 
> > dp83867.c has similar code. However quick grep did not find any
> > mainline users with properties in the MAC node. If that is true, i
> > would suggest removing the code looking in the parent for that phy
> > driver.
> > 
> > Andrew
> > 
> 
> 

BR,
Dinh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: SoCFPGA ethernet broken

[Dinh Nguyen]

On Fri, 16 Oct 2015, Andrew Lunn wrote:

> On Fri, Oct 16, 2015 at 09:38:37AM -0500, Dinh Nguyen wrote:
> > On Fri, 16 Oct 2015, Andrew Lunn wrote:
> > 
> > > > Another debugging point, the SoCFPGA board has a Micrel ksz9021 PHY 
> > > > attached
> > > > to the ethernet port. What I'm seeing is that with 8b63ec1837fa patch, 
> > > > when
> > > > the call to ksz9021_config_init() is made both of_node and 
> > > > dev->parent->of_node
> > > > are NULL, without the patch the dev->parent->of_node is a valid 
> > > > pointer. Thus
> > > > the skew values get programmed to the phy.
> > > 
> > > Ah!
> > > 
> > > You have the phy device tree parameters in the wrong place. These are
> > > phy paramters, so should really be in the phy node. But
> > > socfpga_cyclone5_socdk.dts has them in the MAC node.
> > >
> > 
> > Alright, let me see if I can rework the DTS.
> 
> Well, we are not supposed to break device tree bindings. So we should
> try to make this work again.
>  

Great..thanks for recognizing that point.
 
> > > There is nothing in Documentation/devicetree/bindings/net/micrel.txt
> > > which says you are allowed to place them in the MAC node. Obviously
> > > the code did allow this, which is what has now broken.
> > 
> > I was following Documentation/devicetree/bindings/net/micrel-ksz90x1.txt and
> > in this document I was following the autodetected PHY example. Did I 
> > mis-interpret
> > the example?
> 
> I was looking at the wrong binding documentation. So yes, it is
> documented you can do this.
> 
> But i still think it is wrong. These are phy properties, implemented
> by the phy, so should be in the phy node. So moving them would be
> good. But as i said, we should fix backwards compatibility if
> possible.
>

I moved the phy settings to a separate phy node, but it did not seem to make a
difference at all.

So I think I'll move to inspect what Florian had suggested, and that was to look
at: drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c::stmmac_mdio_register 

BR,
Dinh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: SoCFPGA ethernet broken

[Dinh Nguyen]

On Fri, 16 Oct 2015, Andrew Lunn wrote:

> > Another debugging point, the SoCFPGA board has a Micrel ksz9021 PHY attached
> > to the ethernet port. What I'm seeing is that with 8b63ec1837fa patch, when
> > the call to ksz9021_config_init() is made both of_node and 
> > dev->parent->of_node
> > are NULL, without the patch the dev->parent->of_node is a valid pointer. 
> > Thus
> > the skew values get programmed to the phy.
> 
> Ah!
> 
> You have the phy device tree parameters in the wrong place. These are
> phy paramters, so should really be in the phy node. But
> socfpga_cyclone5_socdk.dts has them in the MAC node.
>

Alright, let me see if I can rework the DTS.
 
> There is nothing in Documentation/devicetree/bindings/net/micrel.txt
> which says you are allowed to place them in the MAC node. Obviously
> the code did allow this, which is what has now broken.
>

I was following Documentation/devicetree/bindings/net/micrel-ksz90x1.txt and
in this document I was following the autodetected PHY example. Did I 
mis-interpret
the example?

BR,
Dinh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: SoCFPGA ethernet broken

[Dinh Nguyen]

On Thu, 15 Oct 2015, Florian Fainelli wrote:

> On 15/10/15 13:49, Dinh Nguyen wrote:
> >>
> >> Does this text change with and without the 8b63ec1837fa patch?
> > 
> > No, this text does not change with/without the 8b63ec1837fa patch.
> 
> Could you instrument mdiobus_scan(), get_phy_device() and
> phy_device_create/register to see if the parent is NULL, non-NULL?
> 

[0.800479] get_phy_device bus=0xeee26c00 addr=0004 phy_id=00221611
[0.806735] phy_device_create dev->dev.parent=0xeee26c50
[0.84] phy_device_register completed
[0.814675] mdiobus_scan phydev=0xeee27000
[1.979034] ksz9021_config_init of_node=

 Without patch 

[0.801294] get_phy_device bus=0xeeef6c00 addr=0004 phy_id=00221611
[0.807551] phy_device_create dev->dev.parent=0xeed6cc10
[0.811929] phy_device_register completed
[0.815510] mdiobus_scan phydev=0xeeef7000
[1.979034] ksz9021_config_init of_node=0xef203538
[1.988064] ksz9021_load_values_from_of
[1.992485] ksz9021_load_values_from_of
[1.996905] ksz9021_load_values_from_of 

BR,
Dinh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: SoCFPGA ethernet broken

[Dinh Nguyen]

On Thu, 15 Oct 2015, Florian Fainelli wrote:

> On 15/10/15 13:49, Dinh Nguyen wrote:
> >>
> >> Does this text change with and without the 8b63ec1837fa patch?
> > 
> > No, this text does not change with/without the 8b63ec1837fa patch.
> 
> Could you instrument mdiobus_scan(), get_phy_device() and
> phy_device_create/register to see if the parent is NULL, non-NULL?
>

Yes, I can do that.
 
> So far, I cannot see what is wrong with David's changes, quite the
> contrary, and if there was something wrong with the PHY device creation,
> it should not get you that far.
> 
> You have not answered my previous question though, do you have PHY
> fixups registered for that ID?

By fixups, do you mean the skew values that are in the device tree?
Those are the only fixups that I have the PHY.

Another debugging point, the SoCFPGA board has a Micrel ksz9021 PHY attached
to the ethernet port. What I'm seeing is that with 8b63ec1837fa patch, when
the call to ksz9021_config_init() is made both of_node and dev->parent->of_node
are NULL, without the patch the dev->parent->of_node is a valid pointer. Thus
the skew values get programmed to the phy.

BR,
Dinh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: SoCFPGA ethernet broken

[Dinh Nguyen]

On 10/15/2015 03:35 PM, David Daney wrote:
> On 10/15/2015 01:25 PM, Florian Fainelli wrote:
>> On 15/10/15 12:59, Dinh Nguyen wrote:
>>> On 10/15/2015 03:03 PM, Florian Fainelli wrote:
>>>> On 15/10/15 12:09, Dinh Nguyen wrote:
>>>>> Hi,
>>>>>
>>>>> commit "8b63ec1837fa phylib: Make PHYs children of their MDIO bus, not
>>>>> the bus' parent." seems to have broken ethernet support for the
>>>>> SoCFPGA
>>>>> platform which is using the stmmac ethernet driver.
>>>>
>>>> It is not clear to me how this relates to what you are seeing yet.
>>>>
>>>>>
>>>>> It appears that during DHCP, it cannot get an IP address. This only
>>>>> happens if ethernet was not used by the bootloader to tftp an kernel
>>>>> image. If I use the bootloader to tftp an image then ethernet is
>>>>> working
>>>>> fine. So I think the PHY is not getting enabled properly.
>>>>>
>>>>> If I revert this patch, then ethernet is back to working on the
>>>>> platform.
>>>>
>>>> Is the Device Tree source for this platform available somewhere to
>>>> look at?
>>>>
>>>
>>> Yes, I'm using the DTS that is in the mainline:
>>>
>>> arch/arm/boot/dts/socfpga.dtsi
>>> arch/arm/boot/dts/socfpga_cyclone5.dtsi
>>> arch/arm/boot/dts/socfpga_cyclone5_socdk.dts
>>
>> There are no PHY devices in any of these DTS files, instead there is the
>> non-standard "phy-addr" property which is set to 0x supposedly
>> to indicate that the MDIO bus should be scanned. This is likely part of
>> your problem. The stmmac driver seems to be looking for "snps,phy-addr"
>> and not "phy-addr", so I am not even clear how this is supposed to work,
>> and the driver mentions this custom property is deprecated anyway.
>>
> 
> I think it is OK not to expose the PHYs in the device tree if they can
> be accurately probed without knowing information from the device tree.
> 
>> The core problem is in
>> drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c::stmmac_mdio_register
>> which manually detects the PHY, that is mostly fine, except that it does
>> not really seem to work here for a reason that is still unclear to me.
>>
> 
> I agree with this analysis.  I have also been looking at the code and
> cannot see anything that depends on what the parent device of the PHY
> is.  So it is a bit mystifying.
> 
> 
> I noticed in your original message you had in the boot log this:
> 
> .
> .
> .
> [0.804992] libphy: stmmac: probed
> [0.808410] eth0: PHY ID 00221611 at 4 IRQ POLL (stmmac-0:04) active
> .
> .
> .
> 
> Does this text change with and without the 8b63ec1837fa patch?

No, this text does not change with/without the 8b63ec1837fa patch.

Dinh

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: SoCFPGA ethernet broken

[Dinh Nguyen]

On 10/15/2015 03:03 PM, Florian Fainelli wrote:
> On 15/10/15 12:09, Dinh Nguyen wrote:
>> Hi,
>>
>> commit "8b63ec1837fa phylib: Make PHYs children of their MDIO bus, not
>> the bus' parent." seems to have broken ethernet support for the SoCFPGA
>> platform which is using the stmmac ethernet driver.
> 
> It is not clear to me how this relates to what you are seeing yet.
> 
>>
>> It appears that during DHCP, it cannot get an IP address. This only
>> happens if ethernet was not used by the bootloader to tftp an kernel
>> image. If I use the bootloader to tftp an image then ethernet is working
>> fine. So I think the PHY is not getting enabled properly.
>>
>> If I revert this patch, then ethernet is back to working on the platform.
> 
> Is the Device Tree source for this platform available somewhere to look at?
> 

Yes, I'm using the DTS that is in the mainline:

arch/arm/boot/dts/socfpga.dtsi
arch/arm/boot/dts/socfpga_cyclone5.dtsi
arch/arm/boot/dts/socfpga_cyclone5_socdk.dts

Dinh


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

SoCFPGA ethernet broken

[Dinh Nguyen]

Hi,

commit "8b63ec1837fa phylib: Make PHYs children of their MDIO bus, not
the bus' parent." seems to have broken ethernet support for the SoCFPGA
platform which is using the stmmac ethernet driver.

It appears that during DHCP, it cannot get an IP address. This only
happens if ethernet was not used by the bootloader to tftp an kernel
image. If I use the bootloader to tftp an image then ethernet is working
fine. So I think the PHY is not getting enabled properly.

If I revert this patch, then ethernet is back to working on the platform.

I just tested this in the v4.3-rc5.

Will continue to debug, but was wondering if anyone has seen this issue.

Thanks for any help/comments.

Dinh

Bootlog below.

[0.00] Booting Linux on physical CPU 0x0
[0.00] Initializing cgroup subsys cpuset
[0.00] Linux version 4.3.0-rc5-00037-g5b5f145
(dinguyen@linux-builds1) (gcc version 4.7.3 20130226 (prerelease) (crosstool
-NG linaro-1.13.1-4.7-2013.03-20130313 - Linaro GCC 2013.03) ) #1 SMP
Thu Oct 15 13:55:48 CDT 2015
[0.00] CPU: ARMv7 Processor [413fc090] revision 0 (ARMv7),
cr=10c5387d
[0.00] CPU: PIPT / VIPT nonaliasing data cache, VIPT aliasing
instruction cache
[0.00] Machine model: Altera SOCFPGA Cyclone V SoC Development Kit
[0.00] Truncating RAM at 0x-0x4000 to -0x2f80
[0.00] Consider using a HIGHMEM enabled kernel.
[0.00] Memory policy: Data cache writealloc
[0.00] On node 0 totalpages: 194560
[0.00] free_area_init_node: node 0, pgdat c067c080, node_mem_map
ef20b000
[0.00]   Normal zone: 1520 pages used for memmap
[0.00]   Normal zone: 0 pages reserved
[0.00]   Normal zone: 194560 pages, LIFO batch:31
[0.00] PERCPU: Embedded 13 pages/cpu @ef1df000 s21760 r8192
d23296 u53248
[0.00] pcpu-alloc: s21760 r8192 d23296 u53248 alloc=13*4096
[0.00] pcpu-alloc: [0] 0 [0] 1
[0.00] Built 1 zonelists in Zone order, mobility grouping on.
Total pages: 193040
[0.00] Kernel command line: console=ttyS0,115200 root=/dev/nfs
rw nfsroot=137.57.160.210:/home/dinguyen/rootfs_yocto ip=dh
cp debug
[0.00] PID hash table entries: 4096 (order: 2, 16384 bytes)
[0.00] Dentry cache hash table entries: 131072 (order: 7, 524288
bytes)
[0.00] Inode-cache hash table entries: 65536 (order: 6, 262144
bytes)
[0.00] Memory: 764368K/778240K available (4696K kernel code,
274K rwdata, 1328K rodata, 324K init, 131K bss, 13872K reserv
ed, 0K cma-reserved)
[0.00] Virtual kernel memory layout:
[0.00] vector  : 0x - 0x1000   (   4 kB)
[0.00] fixmap  : 0xffc0 - 0xfff0   (3072 kB)
[0.00] vmalloc : 0xf000 - 0xff00   ( 240 MB)
[0.00] lowmem  : 0xc000 - 0xef80   ( 760 MB)
[0.00] modules : 0xbf00 - 0xc000   (  16 MB)
[0.00]   .text : 0xc0008000 - 0xc05ea5f4   (6026 kB)
[0.00]   .init : 0xc05eb000 - 0xc063c000   ( 324 kB)
[0.00]   .data : 0xc063c000 - 0xc0680b30   ( 275 kB)
[0.00].bss : 0xc0680b30 - 0xc06a1a38   ( 132 kB)
[0.00] SLUB: HWalign=64, Order=0-3, MinObjects=0, CPUs=2, Nodes=1
[0.00] Hierarchical RCU implementation.
[0.00]  Build-time adjustment of leaf fanout to 32.
[0.00] NR_IRQS:16 nr_irqs:16 16
[0.00] L2C-310 enabling early BRESP for Cortex-A9
[0.00] L2C-310 full line of zeros enabled for Cortex-A9
[0.00] L2C-310 ID prefetch enabled, offset 1 lines
[0.00] L2C-310 dynamic clock gating enabled, standby mode enabled
[0.00] L2C-310 cache controller enabled, 8 ways, 512 kB
[0.00] L2C-310: CACHE_ID 0x410030c9, AUX_CTRL 0x76060001
[0.00] clocksource: timer1: mask: 0x max_cycles:
0x, max_idle_ns: 19112604467 ns
[0.05] sched_clock: 32 bits at 100MHz, resolution 10ns, wraps
every 21474836475ns
[0.000134] Console: colour dummy device 80x30
[0.000151] Calibrating delay loop... 1836.64 BogoMIPS (lpj=9183232)
[0.059867] pid_max: default: 32768 minimum: 301
[0.059938] Mount-cache hash table entries: 2048 (order: 1, 8192 bytes)
[0.059947] Mountpoint-cache hash table entries: 2048 (order: 1, 8192
bytes)
[0.060411] CPU: Testing write buffer coherency: ok
[0.060586] CPU0: thread -1, cpu 0, socket 0, mpidr 8000
[0.060735] Setting up static identity map for 0x8280 - 0x82d8
[0.119895] CPU1: thread -1, cpu 1, socket 0, mpidr 8001
[0.119954] Brought up 2 CPUs
[0.119968] SMP: Total of 2 processors activated (3679.84 BogoMIPS).
[0.119974] CPU: All CPU(s) started in SVC mode.
[0.120594] devtmpfs: initialized
[0.124057] VFP support v0.3: implementor 41 architecture 3 part 30
variant 9 rev 4
[0.124296] clocksource: jiffies: mask: 0x max_cycles:
0x, max_idle_ns: 1911260446275 ns
[0.125249] NET: Registered protocol family 16

Re: SoCFPGA ethernet broken

[Dinh Nguyen]

On Thu, 15 Oct 2015, Florian Fainelli wrote:

> On 15/10/15 13:49, Dinh Nguyen wrote:
> >>
> >> Does this text change with and without the 8b63ec1837fa patch?
> > 
> > No, this text does not change with/without the 8b63ec1837fa patch.
> 
> Could you instrument mdiobus_scan(), get_phy_device() and
> phy_device_create/register to see if the parent is NULL, non-NULL?
>

Yes, I can do that.
 
> So far, I cannot see what is wrong with David's changes, quite the
> contrary, and if there was something wrong with the PHY device creation,
> it should not get you that far.
> 
> You have not answered my previous question though, do you have PHY
> fixups registered for that ID?

By fixups, do you mean the skew values that are in the device tree?
Those are the only fixups that I have the PHY.

Another debugging point, the SoCFPGA board has a Micrel ksz9021 PHY attached
to the ethernet port. What I'm seeing is that with 8b63ec1837fa patch, when
the call to ksz9021_config_init() is made both of_node and dev->parent->of_node
are NULL, without the patch the dev->parent->of_node is a valid pointer. Thus
the skew values get programmed to the phy.

BR,
Dinh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: SoCFPGA ethernet broken

[Dinh Nguyen]

On Thu, 15 Oct 2015, Florian Fainelli wrote:

> On 15/10/15 13:49, Dinh Nguyen wrote:
> >>
> >> Does this text change with and without the 8b63ec1837fa patch?
> > 
> > No, this text does not change with/without the 8b63ec1837fa patch.
> 
> Could you instrument mdiobus_scan(), get_phy_device() and
> phy_device_create/register to see if the parent is NULL, non-NULL?
> 

[0.800479] get_phy_device bus=0xeee26c00 addr=0004 phy_id=00221611
[0.806735] phy_device_create dev->dev.parent=0xeee26c50
[0.84] phy_device_register completed
[0.814675] mdiobus_scan phydev=0xeee27000
[1.979034] ksz9021_config_init of_node=

 Without patch 

[0.801294] get_phy_device bus=0xeeef6c00 addr=0004 phy_id=00221611
[0.807551] phy_device_create dev->dev.parent=0xeed6cc10
[0.811929] phy_device_register completed
[0.815510] mdiobus_scan phydev=0xeeef7000
[1.979034] ksz9021_config_init of_node=0xef203538
[1.988064] ksz9021_load_values_from_of
[1.992485] ksz9021_load_values_from_of
[1.996905] ksz9021_load_values_from_of 

BR,
Dinh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: SoCFPGA ethernet broken

[Dinh Nguyen]

On 10/15/2015 03:03 PM, Florian Fainelli wrote:
> On 15/10/15 12:09, Dinh Nguyen wrote:
>> Hi,
>>
>> commit "8b63ec1837fa phylib: Make PHYs children of their MDIO bus, not
>> the bus' parent." seems to have broken ethernet support for the SoCFPGA
>> platform which is using the stmmac ethernet driver.
> 
> It is not clear to me how this relates to what you are seeing yet.
> 
>>
>> It appears that during DHCP, it cannot get an IP address. This only
>> happens if ethernet was not used by the bootloader to tftp an kernel
>> image. If I use the bootloader to tftp an image then ethernet is working
>> fine. So I think the PHY is not getting enabled properly.
>>
>> If I revert this patch, then ethernet is back to working on the platform.
> 
> Is the Device Tree source for this platform available somewhere to look at?
> 

Yes, I'm using the DTS that is in the mainline:

arch/arm/boot/dts/socfpga.dtsi
arch/arm/boot/dts/socfpga_cyclone5.dtsi
arch/arm/boot/dts/socfpga_cyclone5_socdk.dts

Dinh


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: SoCFPGA ethernet broken

[Dinh Nguyen]

On 10/15/2015 03:35 PM, David Daney wrote:
> On 10/15/2015 01:25 PM, Florian Fainelli wrote:
>> On 15/10/15 12:59, Dinh Nguyen wrote:
>>> On 10/15/2015 03:03 PM, Florian Fainelli wrote:
>>>> On 15/10/15 12:09, Dinh Nguyen wrote:
>>>>> Hi,
>>>>>
>>>>> commit "8b63ec1837fa phylib: Make PHYs children of their MDIO bus, not
>>>>> the bus' parent." seems to have broken ethernet support for the
>>>>> SoCFPGA
>>>>> platform which is using the stmmac ethernet driver.
>>>>
>>>> It is not clear to me how this relates to what you are seeing yet.
>>>>
>>>>>
>>>>> It appears that during DHCP, it cannot get an IP address. This only
>>>>> happens if ethernet was not used by the bootloader to tftp an kernel
>>>>> image. If I use the bootloader to tftp an image then ethernet is
>>>>> working
>>>>> fine. So I think the PHY is not getting enabled properly.
>>>>>
>>>>> If I revert this patch, then ethernet is back to working on the
>>>>> platform.
>>>>
>>>> Is the Device Tree source for this platform available somewhere to
>>>> look at?
>>>>
>>>
>>> Yes, I'm using the DTS that is in the mainline:
>>>
>>> arch/arm/boot/dts/socfpga.dtsi
>>> arch/arm/boot/dts/socfpga_cyclone5.dtsi
>>> arch/arm/boot/dts/socfpga_cyclone5_socdk.dts
>>
>> There are no PHY devices in any of these DTS files, instead there is the
>> non-standard "phy-addr" property which is set to 0x supposedly
>> to indicate that the MDIO bus should be scanned. This is likely part of
>> your problem. The stmmac driver seems to be looking for "snps,phy-addr"
>> and not "phy-addr", so I am not even clear how this is supposed to work,
>> and the driver mentions this custom property is deprecated anyway.
>>
> 
> I think it is OK not to expose the PHYs in the device tree if they can
> be accurately probed without knowing information from the device tree.
> 
>> The core problem is in
>> drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c::stmmac_mdio_register
>> which manually detects the PHY, that is mostly fine, except that it does
>> not really seem to work here for a reason that is still unclear to me.
>>
> 
> I agree with this analysis.  I have also been looking at the code and
> cannot see anything that depends on what the parent device of the PHY
> is.  So it is a bit mystifying.
> 
> 
> I noticed in your original message you had in the boot log this:
> 
> .
> .
> .
> [0.804992] libphy: stmmac: probed
> [0.808410] eth0: PHY ID 00221611 at 4 IRQ POLL (stmmac-0:04) active
> .
> .
> .
> 
> Does this text change with and without the 8b63ec1837fa patch?

No, this text does not change with/without the 8b63ec1837fa patch.

Dinh

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

SoCFPGA ethernet broken

[Dinh Nguyen]

Hi,

commit "8b63ec1837fa phylib: Make PHYs children of their MDIO bus, not
the bus' parent." seems to have broken ethernet support for the SoCFPGA
platform which is using the stmmac ethernet driver.

It appears that during DHCP, it cannot get an IP address. This only
happens if ethernet was not used by the bootloader to tftp an kernel
image. If I use the bootloader to tftp an image then ethernet is working
fine. So I think the PHY is not getting enabled properly.

If I revert this patch, then ethernet is back to working on the platform.

I just tested this in the v4.3-rc5.

Will continue to debug, but was wondering if anyone has seen this issue.

Thanks for any help/comments.

Dinh

Bootlog below.

[0.00] Booting Linux on physical CPU 0x0
[0.00] Initializing cgroup subsys cpuset
[0.00] Linux version 4.3.0-rc5-00037-g5b5f145
(dinguyen@linux-builds1) (gcc version 4.7.3 20130226 (prerelease) (crosstool
-NG linaro-1.13.1-4.7-2013.03-20130313 - Linaro GCC 2013.03) ) #1 SMP
Thu Oct 15 13:55:48 CDT 2015
[0.00] CPU: ARMv7 Processor [413fc090] revision 0 (ARMv7),
cr=10c5387d
[0.00] CPU: PIPT / VIPT nonaliasing data cache, VIPT aliasing
instruction cache
[0.00] Machine model: Altera SOCFPGA Cyclone V SoC Development Kit
[0.00] Truncating RAM at 0x-0x4000 to -0x2f80
[0.00] Consider using a HIGHMEM enabled kernel.
[0.00] Memory policy: Data cache writealloc
[0.00] On node 0 totalpages: 194560
[0.00] free_area_init_node: node 0, pgdat c067c080, node_mem_map
ef20b000
[0.00]   Normal zone: 1520 pages used for memmap
[0.00]   Normal zone: 0 pages reserved
[0.00]   Normal zone: 194560 pages, LIFO batch:31
[0.00] PERCPU: Embedded 13 pages/cpu @ef1df000 s21760 r8192
d23296 u53248
[0.00] pcpu-alloc: s21760 r8192 d23296 u53248 alloc=13*4096
[0.00] pcpu-alloc: [0] 0 [0] 1
[0.00] Built 1 zonelists in Zone order, mobility grouping on.
Total pages: 193040
[0.00] Kernel command line: console=ttyS0,115200 root=/dev/nfs
rw nfsroot=137.57.160.210:/home/dinguyen/rootfs_yocto ip=dh
cp debug
[0.00] PID hash table entries: 4096 (order: 2, 16384 bytes)
[0.00] Dentry cache hash table entries: 131072 (order: 7, 524288
bytes)
[0.00] Inode-cache hash table entries: 65536 (order: 6, 262144
bytes)
[0.00] Memory: 764368K/778240K available (4696K kernel code,
274K rwdata, 1328K rodata, 324K init, 131K bss, 13872K reserv
ed, 0K cma-reserved)
[0.00] Virtual kernel memory layout:
[0.00] vector  : 0x - 0x1000   (   4 kB)
[0.00] fixmap  : 0xffc0 - 0xfff0   (3072 kB)
[0.00] vmalloc : 0xf000 - 0xff00   ( 240 MB)
[0.00] lowmem  : 0xc000 - 0xef80   ( 760 MB)
[0.00] modules : 0xbf00 - 0xc000   (  16 MB)
[0.00]   .text : 0xc0008000 - 0xc05ea5f4   (6026 kB)
[0.00]   .init : 0xc05eb000 - 0xc063c000   ( 324 kB)
[0.00]   .data : 0xc063c000 - 0xc0680b30   ( 275 kB)
[0.00].bss : 0xc0680b30 - 0xc06a1a38   ( 132 kB)
[0.00] SLUB: HWalign=64, Order=0-3, MinObjects=0, CPUs=2, Nodes=1
[0.00] Hierarchical RCU implementation.
[0.00]  Build-time adjustment of leaf fanout to 32.
[0.00] NR_IRQS:16 nr_irqs:16 16
[0.00] L2C-310 enabling early BRESP for Cortex-A9
[0.00] L2C-310 full line of zeros enabled for Cortex-A9
[0.00] L2C-310 ID prefetch enabled, offset 1 lines
[0.00] L2C-310 dynamic clock gating enabled, standby mode enabled
[0.00] L2C-310 cache controller enabled, 8 ways, 512 kB
[0.00] L2C-310: CACHE_ID 0x410030c9, AUX_CTRL 0x76060001
[0.00] clocksource: timer1: mask: 0x max_cycles:
0x, max_idle_ns: 19112604467 ns
[0.05] sched_clock: 32 bits at 100MHz, resolution 10ns, wraps
every 21474836475ns
[0.000134] Console: colour dummy device 80x30
[0.000151] Calibrating delay loop... 1836.64 BogoMIPS (lpj=9183232)
[0.059867] pid_max: default: 32768 minimum: 301
[0.059938] Mount-cache hash table entries: 2048 (order: 1, 8192 bytes)
[0.059947] Mountpoint-cache hash table entries: 2048 (order: 1, 8192
bytes)
[0.060411] CPU: Testing write buffer coherency: ok
[0.060586] CPU0: thread -1, cpu 0, socket 0, mpidr 8000
[0.060735] Setting up static identity map for 0x8280 - 0x82d8
[0.119895] CPU1: thread -1, cpu 1, socket 0, mpidr 8001
[0.119954] Brought up 2 CPUs
[0.119968] SMP: Total of 2 processors activated (3679.84 BogoMIPS).
[0.119974] CPU: All CPU(s) started in SVC mode.
[0.120594] devtmpfs: initialized
[0.124057] VFP support v0.3: implementor 41 architecture 3 part 30
variant 9 rev 4
[0.124296] clocksource: jiffies: mask: 0x max_cycles:
0x, max_idle_ns: 1911260446275 ns
[0.125249] NET: Registered protocol family 16

Re: [PATCH v2] ARM: socfpga: dts: add fpga manager

[Dinh Nguyen]

On Tue, 13 Oct 2015, Steffen Trumtrar wrote:

> On Tue, Oct 13, 2015 at 02:51:28PM -0500, Dinh Nguyen wrote:
> > On Tue, 13 Oct 2015, at...@opensource.altera.com wrote:
> > 
> > > From: Alan Tull 
> > > 
> > > Add FPGA manager to device tree for SoCFPGA.
> > > 
> > > Signed-off-by: Alan Tull 
> > > ---
> > > v2: Remove 0x after @
> > > No caps in hex numbers
> > > renamed hps_0_fpgamgr to fpgamgr0
> > > move node to be in alpha order by node name
> > > ---
> > >  arch/arm/boot/dts/socfpga.dtsi | 7 +++
> > >  1 file changed, 7 insertions(+)
> > > 
> > 
> > Applied!
> 
> What about the
> 
> "Also, you need to document the "altr,socfpga-fpga-mgr"."
> 
> you mentioned in v1 ???
>

It was my oversight. It has already been documented. 

BR,
Dinh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2] ARM: socfpga: dts: add fpga manager

[Dinh Nguyen]

On Tue, 13 Oct 2015, at...@opensource.altera.com wrote:

> From: Alan Tull 
> 
> Add FPGA manager to device tree for SoCFPGA.
> 
> Signed-off-by: Alan Tull 
> ---
> v2: Remove 0x after @
> No caps in hex numbers
> renamed hps_0_fpgamgr to fpgamgr0
> move node to be in alpha order by node name
> ---
>  arch/arm/boot/dts/socfpga.dtsi | 7 +++
>  1 file changed, 7 insertions(+)
> 

Applied!

Thanks,
Dinh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] ARM: socfpga_defconfig: enable fpga manager

[Dinh Nguyen]

On Tue, 13 Oct 2015, at...@opensource.altera.com wrote:

> From: Alan Tull 
> 
> Enable fpga manager framework and low level driver for
> socfpga in socfpga_defconfig
> 
> Signed-off-by: Alan Tull 
> ---
>  arch/arm/configs/socfpga_defconfig | 2 ++
>  1 file changed, 2 insertions(+)
> 

Applied!

Thanks,
Dinh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] ARM: socfpga: dts: add fpga manager

[Dinh Nguyen]

On Tue, 13 Oct 2015, Steffen Trumtrar wrote:

> Hi Alan!
> 
> On Tue, Oct 13, 2015 at 01:28:20PM -0500, at...@opensource.altera.com wrote:
> > From: Alan Tull 
> > 
> > Add FPGA manager to device tree for SoCFPGA.
> > 
> > Signed-off-by: Alan Tull 
> > ---
> >  arch/arm/boot/dts/socfpga.dtsi | 7 +++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi
> > index 314e589..e37ba11 100644
> > --- a/arch/arm/boot/dts/socfpga.dtsi
> > +++ b/arch/arm/boot/dts/socfpga.dtsi
> > @@ -834,5 +834,12 @@
> > compatible = "altr,sys-mgr", "syscon";
> > reg = <0xffd08000 0x4000>;
> > };
> > +
> > +   hps_0_fpgamgr: fpgamgr@0xff706000 {
> > +   compatible = "altr,socfpga-fpga-mgr";
> > +   reg = <0xFF706000 0x1000
> > +  0xFFB9 0x1000>;
> > +   interrupts = <0 175 4>;
> > +   };
> > };
> >  };
> 
> Please keep the format in line with the rest of the file:
> Hexvalues are lowercase and no 0x after the "fpgamgr@".
> I'd say "And please put the node in its proper position
> according to its address", but I see just now, that the
> dtsi is all messed up right now anyway :-(
> 

How so? Patches are welcomed!

Please fix the "fpgamgr@" and lower case the hex values. But I would prefer the
nodes to be in alphabetical order.

Also, you need to document the "altr,socfpga-fpga-mgr".

Thanks,
Dinh
> Regards,
> Steffen
> 
> -- 
> Pengutronix e.K.   | |
> Industrial Linux Solutions | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
> Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
> 

BR,
Dinh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2] ARM: socfpga: dts: add fpga manager

[Dinh Nguyen]

On Tue, 13 Oct 2015, at...@opensource.altera.com wrote:

> From: Alan Tull 
> 
> Add FPGA manager to device tree for SoCFPGA.
> 
> Signed-off-by: Alan Tull 
> ---
> v2: Remove 0x after @
> No caps in hex numbers
> renamed hps_0_fpgamgr to fpgamgr0
> move node to be in alpha order by node name
> ---
>  arch/arm/boot/dts/socfpga.dtsi | 7 +++
>  1 file changed, 7 insertions(+)
> 

Applied!

Thanks,
Dinh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] ARM: socfpga: dts: add fpga manager

[Dinh Nguyen]

On Tue, 13 Oct 2015, Steffen Trumtrar wrote:

> Hi Alan!
> 
> On Tue, Oct 13, 2015 at 01:28:20PM -0500, at...@opensource.altera.com wrote:
> > From: Alan Tull 
> > 
> > Add FPGA manager to device tree for SoCFPGA.
> > 
> > Signed-off-by: Alan Tull 
> > ---
> >  arch/arm/boot/dts/socfpga.dtsi | 7 +++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi
> > index 314e589..e37ba11 100644
> > --- a/arch/arm/boot/dts/socfpga.dtsi
> > +++ b/arch/arm/boot/dts/socfpga.dtsi
> > @@ -834,5 +834,12 @@
> > compatible = "altr,sys-mgr", "syscon";
> > reg = <0xffd08000 0x4000>;
> > };
> > +
> > +   hps_0_fpgamgr: fpgamgr@0xff706000 {
> > +   compatible = "altr,socfpga-fpga-mgr";
> > +   reg = <0xFF706000 0x1000
> > +  0xFFB9 0x1000>;
> > +   interrupts = <0 175 4>;
> > +   };
> > };
> >  };
> 
> Please keep the format in line with the rest of the file:
> Hexvalues are lowercase and no 0x after the "fpgamgr@".
> I'd say "And please put the node in its proper position
> according to its address", but I see just now, that the
> dtsi is all messed up right now anyway :-(
> 

How so? Patches are welcomed!

Please fix the "fpgamgr@" and lower case the hex values. But I would prefer the
nodes to be in alphabetical order.

Also, you need to document the "altr,socfpga-fpga-mgr".

Thanks,
Dinh
> Regards,
> Steffen
> 
> -- 
> Pengutronix e.K.   | |
> Industrial Linux Solutions | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
> Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
> 

BR,
Dinh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] ARM: socfpga_defconfig: enable fpga manager

[Dinh Nguyen]

On Tue, 13 Oct 2015, at...@opensource.altera.com wrote:

> From: Alan Tull 
> 
> Enable fpga manager framework and low level driver for
> socfpga in socfpga_defconfig
> 
> Signed-off-by: Alan Tull 
> ---
>  arch/arm/configs/socfpga_defconfig | 2 ++
>  1 file changed, 2 insertions(+)
> 

Applied!

Thanks,
Dinh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2] ARM: socfpga: dts: add fpga manager

[Dinh Nguyen]

On Tue, 13 Oct 2015, Steffen Trumtrar wrote:

> On Tue, Oct 13, 2015 at 02:51:28PM -0500, Dinh Nguyen wrote:
> > On Tue, 13 Oct 2015, at...@opensource.altera.com wrote:
> > 
> > > From: Alan Tull <at...@opensource.altera.com>
> > > 
> > > Add FPGA manager to device tree for SoCFPGA.
> > > 
> > > Signed-off-by: Alan Tull <at...@opensource.altera.com>
> > > ---
> > > v2: Remove 0x after @
> > > No caps in hex numbers
> > > renamed hps_0_fpgamgr to fpgamgr0
> > > move node to be in alpha order by node name
> > > ---
> > >  arch/arm/boot/dts/socfpga.dtsi | 7 +++
> > >  1 file changed, 7 insertions(+)
> > > 
> > 
> > Applied!
> 
> What about the
> 
> "Also, you need to document the "altr,socfpga-fpga-mgr"."
> 
> you mentioned in v1 ???
>

It was my oversight. It has already been documented. 

BR,
Dinh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCHv2] arm64: dts: Add base stratix 10 dtsi

[Dinh Nguyen]

On 08/20/2015 12:23 PM, Mark Rutland wrote:
> Hi,
> 
>> +/ {
>> +compatible = "altr,socfpga-stratix10";
>> +#address-cells = <1>;
>> +#size-cells = <1>;
> 
> I would recommend that you make your root #address-cells and #size-cells equal
> to 2, as that will simplify matters later if/when you need to add anything
> beyond the first 4GB for some particular board.
> 
> If everything in the SoC falls within the first 4GB you can have a ranges
> property on your /soc node and have only one cell below that.
> 

Ok, will update.

> [...]
> 
>> +intc: intc@8000 {
> 
> The unit-address doesn't match the first address in the reg entry.
> 

Right, according to the GIC-400 r0p1 TRM, section 3.2 register map,
shows that

0x - 0x0fff Reserved
0x1000 - 0x1fff Distibutor
...

So even though the GIC address starts at 0x8000, the first usable
register is at 0x9000. I'll have to ask the hw folks if the
0x8000 represents the distributor or the reserved block.

>> +compatible = "arm,gic-400", "arm,cortex-a15-gic";
>> +#interrupt-cells = <3>;
>> +interrupt-controller;
>> +reg = <0x0 0x9000 0x1000>,
>> +  <0x0 0xa000 0x2000>,
>> +  <0x0 0xc000 0x1000>,
>> +  <0x0 0xd000 0x1000>;
>> +};
> 
> Shouldn't the virtual CPU interface also be 0x2000 long?
> 

Yes, it is. Will update

Thanks for reviewing.

Dinh

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCHv2] arm64: dts: Add base stratix 10 dtsi

[Dinh Nguyen]

On 08/20/2015 12:23 PM, Mark Rutland wrote:
 Hi,
 
 +/ {
 +compatible = altr,socfpga-stratix10;
 +#address-cells = 1;
 +#size-cells = 1;
 
 I would recommend that you make your root #address-cells and #size-cells equal
 to 2, as that will simplify matters later if/when you need to add anything
 beyond the first 4GB for some particular board.
 
 If everything in the SoC falls within the first 4GB you can have a ranges
 property on your /soc node and have only one cell below that.
 

Ok, will update.

 [...]
 
 +intc: intc@8000 {
 
 The unit-address doesn't match the first address in the reg entry.
 

Right, according to the GIC-400 r0p1 TRM, section 3.2 register map,
shows that

0x - 0x0fff Reserved
0x1000 - 0x1fff Distibutor
...

So even though the GIC address starts at 0x8000, the first usable
register is at 0x9000. I'll have to ask the hw folks if the
0x8000 represents the distributor or the reserved block.

 +compatible = arm,gic-400, arm,cortex-a15-gic;
 +#interrupt-cells = 3;
 +interrupt-controller;
 +reg = 0x0 0x9000 0x1000,
 +  0x0 0xa000 0x2000,
 +  0x0 0xc000 0x1000,
 +  0x0 0xd000 0x1000;
 +};
 
 Shouldn't the virtual CPU interface also be 0x2000 long?
 

Yes, it is. Will update

Thanks for reviewing.

Dinh

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

Re: [PATCH v4 4/5] Documentation: dt-bindings: pci: altera pcie device tree binding

[Dinh Nguyen]

On Mon, Aug 17, 2015 at 4:09 AM, Ley Foon Tan  wrote:
> This patch adds the bindings for Altera PCIe host controller driver and
> Altera PCIe MSI driver.
>
> Signed-off-by: Ley Foon Tan 
> ---
>  .../devicetree/bindings/pci/altera-pcie-msi.txt| 27 
>  .../devicetree/bindings/pci/altera-pcie.txt| 49 
> ++
>  2 files changed, 76 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pci/altera-pcie-msi.txt
>  create mode 100644 Documentation/devicetree/bindings/pci/altera-pcie.txt
>
> diff --git a/Documentation/devicetree/bindings/pci/altera-pcie-msi.txt 
> b/Documentation/devicetree/bindings/pci/altera-pcie-msi.txt
> new file mode 100644
> index 000..7f330c9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/altera-pcie-msi.txt
> @@ -0,0 +1,27 @@
> +* Altera PCIe MSI controller
> +
> +Required properties:
> +- compatible:  should contain "altr,msi-1.0"
> +- reg: specifies the physical base address of the controller and
> +   the length of the memory mapped region.
> +- reg-names:   Must include the following entries:
> +   "csr": CSR registers
> +   "vector_slave": vectors region

Can you be a little bit more specific here? What is a vectors region?
Also, I'm not 100% sure, but I think "vector-slave" should be used.

> +-interrupts:   specifies the interrupt source of the parent interrupt
> +   controller. The format of the interrupt specifier depends on 
> the
> +   parent interrupt controller.

Need to document "interrupt-parent" .

> +- num-vectors: Number of vectors, range 1 to 32.
> +- msi-controller:  indicates that this is MSI controller node
> +
> +
> +Example
> +msi0: msi@0xFF20 {
> +   compatible = "altr,msi-1.0";
> +   reg = <0xFF20 0x0010
> +   0xFF200010 0x0080>;
> +   reg-names = "csr", "vector_slave";
> +   interrupt-parent = <_0_arm_gic_0>;
> +   interrupts = <0 42 4>;
> +   msi-controller = <1>;
> +   num-vectors = <32>;
> +};
> diff --git a/Documentation/devicetree/bindings/pci/altera-pcie.txt 
> b/Documentation/devicetree/bindings/pci/altera-pcie.txt
> new file mode 100644
> index 000..73a8dc0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/altera-pcie.txt
> @@ -0,0 +1,49 @@
> +* Altera PCIe controller
> +
> +Required properties:
> +- compatible : should contain "altr,pcie-root-port-1.0"
> +- reg: A list of physical base address and length for TXS and CRA.
> +- reg-names:   Must include the following entries:
> +   "Txs" or "txs": TXS region

What is a TXS region?

> +   "Cra" or "cra": Control register access region
> +-interrupts:   specifies the interrupt source of the parent interrupt 
> controller.
> +   The format of the interrupt specifier depends on the parent 
> interrupt
> +   controller.
> +- device_type: must be "pci"
> +- #address-cells:  set to <3>
> +- #size-cells: set to <2>
> +- #interrupt-cells:set to <1>
> +- ranges:  Describes the translation of addresses for root ports 
> and standard
> +   PCI regions.
> +- interrupt-map-mask and interrupt-map: standard PCI properties
> +   to define the mapping of the PCIe interface to interrupt
> +   numbers.

"interrupt-parent" ?

Dinh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v4 2/5] pci:host: Add Altera PCIe host controller driver

[Dinh Nguyen]

On Mon, Aug 17, 2015 at 4:09 AM, Ley Foon Tan  wrote:
> This patch adds the Altera PCIe host controller driver.
>
> Signed-off-by: Ley Foon Tan 
> ---
>  drivers/pci/host/Kconfig   |   7 +
>  drivers/pci/host/Makefile  |   1 +
>  drivers/pci/host/pcie-altera.c | 543 
> +
>  3 files changed, 551 insertions(+)
>  create mode 100644 drivers/pci/host/pcie-altera.c
>
> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
> index 675c2d1..4b4754a 100644
> --- a/drivers/pci/host/Kconfig
> +++ b/drivers/pci/host/Kconfig
> @@ -145,4 +145,11 @@ config PCIE_IPROC_BCMA
>   Say Y here if you want to use the Broadcom iProc PCIe controller
>   through the BCMA bus interface
>



> +
> +/* Address translation table entry size */
> +#define ATT_ENTRY_SIZE 8
> +
> +#define DWORD_MASK 3
> +
> +struct altera_pcie {
> +   struct platform_device  *pdev;
> +   struct resource *txs;

You have "Txs" documented in the bindings document, you have a pointer
here, but you've never used it
anywhre in the code? What is it for?

> +   void __iomem*cra_base;
> +   int irq;
> +   u8  root_bus_nr;
> +   struct irq_domain   *irq_domain;
> +   struct resource bus_range;
> +   struct list_headresources;
> +};
> +



> +
> +static int tlp_cfg_dword_read(struct altera_pcie *pcie, u8 bus, u32 devfn,
> + int where, u32 *value)
> +{
> +   int ret;
> +   u32 headers[TLP_HDR_SIZE];
> +
> +   if (bus == pcie->root_bus_nr)
> +   headers[0] = TLP_CFG_DW0(TLP_FMTTYPE_CFGRD0);
> +   else
> +   headers[0] = TLP_CFG_DW0(TLP_FMTTYPE_CFGRD1);
> +
> +   headers[1] = TLP_CFG_DW1(TLP_REQ_ID(pcie->root_bus_nr, devfn),
> +   TLP_READ_TAG);
> +   headers[2] = TLP_CFG_DW2(bus, devfn, where);
> +
> +   tlp_write_packet(pcie, headers, 0);
> +
> +   ret = tlp_read_packet(pcie, value);
> +   if (ret)
> +   *value = ~0UL;  /* return 0x if error */
> +
> +   return ret;
> +}
> +
> +static int tlp_cfg_dword_write(struct altera_pcie *pcie, u8 bus, u32 devfn,
> +  int where, u32 value)
> +{
> +   u32 headers[TLP_HDR_SIZE];
> +
> +   if (bus == pcie->root_bus_nr)
> +   headers[0] = TLP_CFG_DW0(TLP_FMTTYPE_CFGWR0);
> +   else
> +   headers[0] = TLP_CFG_DW0(TLP_FMTTYPE_CFGWR1);
> +
> +   headers[1] = TLP_CFG_DW1(TLP_REQ_ID(pcie->root_bus_nr, devfn),
> +   TLP_WRITE_TAG);
> +   headers[2] = TLP_CFG_DW2(bus, devfn, where);
> +
> +   tlp_write_packet(pcie, headers, value);
> +
> +   tlp_read_packet(pcie, NULL);

You need to check for the error here.

> +
> +   /* Keep an eye out for changes to the root bus number */
> +   if ((bus == pcie->root_bus_nr) && (where == PCI_PRIMARY_BUS))
> +   pcie->root_bus_nr = (u8)(value);
> +
> +   return PCIBIOS_SUCCESSFUL;
> +}
> +



> +
> +static int altera_pcie_parse_dt(struct altera_pcie *pcie)
> +{
> +   struct resource *cra;
> +   struct platform_device *pdev = pcie->pdev;
> +
> +   cra = platform_get_resource_byname(pdev, IORESOURCE_MEM, "Cra");
> +   if (!cra) {
> +   cra = platform_get_resource_byname(pdev, IORESOURCE_MEM, 
> "cra");
> +   if (!cra) {
> +   dev_err(>dev,
> +   "no cra memory resource defined\n");
> +   return -ENODEV;
> +   }
> +   }
> +

What about "Txs"?

Dinh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCHv2] arm64: dts: Add base stratix 10 dtsi

[Dinh Nguyen]

On Tue, 11 Aug 2015, dingu...@opensource.altera.com wrote:

> From: Dinh Nguyen 
> 
> Add the base DTS for Altera's SoCFPGA Stratix 10 platform.
> 
> Signed-off-by: Dinh Nguyen 
> ---
> v2: use interrupt-affinity for pmu node
> ---
>  arch/arm64/Kconfig |   5 +
>  arch/arm64/boot/dts/Makefile   |   1 +
>  arch/arm64/boot/dts/altera/Makefile|   5 +
>  arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi  | 358 
> +
>  .../boot/dts/altera/socfpga_stratix10_socdk.dts|  38 +++
>  arch/arm64/configs/defconfig   |   1 +
>  6 files changed, 408 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/altera/Makefile
>  create mode 100644 arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
>  create mode 100644 arch/arm64/boot/dts/altera/socfpga_stratix10_socdk.dts
> 

Soft ping on this pach?

BR,
Dinh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v4 4/5] Documentation: dt-bindings: pci: altera pcie device tree binding

[Dinh Nguyen]

On Mon, Aug 17, 2015 at 4:09 AM, Ley Foon Tan lf...@altera.com wrote:
 This patch adds the bindings for Altera PCIe host controller driver and
 Altera PCIe MSI driver.

 Signed-off-by: Ley Foon Tan lf...@altera.com
 ---
  .../devicetree/bindings/pci/altera-pcie-msi.txt| 27 
  .../devicetree/bindings/pci/altera-pcie.txt| 49 
 ++
  2 files changed, 76 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/pci/altera-pcie-msi.txt
  create mode 100644 Documentation/devicetree/bindings/pci/altera-pcie.txt

 diff --git a/Documentation/devicetree/bindings/pci/altera-pcie-msi.txt 
 b/Documentation/devicetree/bindings/pci/altera-pcie-msi.txt
 new file mode 100644
 index 000..7f330c9
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/pci/altera-pcie-msi.txt
 @@ -0,0 +1,27 @@
 +* Altera PCIe MSI controller
 +
 +Required properties:
 +- compatible:  should contain altr,msi-1.0
 +- reg: specifies the physical base address of the controller and
 +   the length of the memory mapped region.
 +- reg-names:   Must include the following entries:
 +   csr: CSR registers
 +   vector_slave: vectors region

Can you be a little bit more specific here? What is a vectors region?
Also, I'm not 100% sure, but I think vector-slave should be used.

 +-interrupts:   specifies the interrupt source of the parent interrupt
 +   controller. The format of the interrupt specifier depends on 
 the
 +   parent interrupt controller.

Need to document interrupt-parent .

 +- num-vectors: Number of vectors, range 1 to 32.
 +- msi-controller:  indicates that this is MSI controller node
 +
 +
 +Example
 +msi0: msi@0xFF20 {
 +   compatible = altr,msi-1.0;
 +   reg = 0xFF20 0x0010
 +   0xFF200010 0x0080;
 +   reg-names = csr, vector_slave;
 +   interrupt-parent = hps_0_arm_gic_0;
 +   interrupts = 0 42 4;
 +   msi-controller = 1;
 +   num-vectors = 32;
 +};
 diff --git a/Documentation/devicetree/bindings/pci/altera-pcie.txt 
 b/Documentation/devicetree/bindings/pci/altera-pcie.txt
 new file mode 100644
 index 000..73a8dc0
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/pci/altera-pcie.txt
 @@ -0,0 +1,49 @@
 +* Altera PCIe controller
 +
 +Required properties:
 +- compatible : should contain altr,pcie-root-port-1.0
 +- reg: A list of physical base address and length for TXS and CRA.
 +- reg-names:   Must include the following entries:
 +   Txs or txs: TXS region

What is a TXS region?

 +   Cra or cra: Control register access region
 +-interrupts:   specifies the interrupt source of the parent interrupt 
 controller.
 +   The format of the interrupt specifier depends on the parent 
 interrupt
 +   controller.
 +- device_type: must be pci
 +- #address-cells:  set to 3
 +- #size-cells: set to 2
 +- #interrupt-cells:set to 1
 +- ranges:  Describes the translation of addresses for root ports 
 and standard
 +   PCI regions.
 +- interrupt-map-mask and interrupt-map: standard PCI properties
 +   to define the mapping of the PCIe interface to interrupt
 +   numbers.

interrupt-parent ?

Dinh
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v4 2/5] pci:host: Add Altera PCIe host controller driver

[Dinh Nguyen]

On Mon, Aug 17, 2015 at 4:09 AM, Ley Foon Tan lf...@altera.com wrote:
 This patch adds the Altera PCIe host controller driver.

 Signed-off-by: Ley Foon Tan lf...@altera.com
 ---
  drivers/pci/host/Kconfig   |   7 +
  drivers/pci/host/Makefile  |   1 +
  drivers/pci/host/pcie-altera.c | 543 
 +
  3 files changed, 551 insertions(+)
  create mode 100644 drivers/pci/host/pcie-altera.c

 diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
 index 675c2d1..4b4754a 100644
 --- a/drivers/pci/host/Kconfig
 +++ b/drivers/pci/host/Kconfig
 @@ -145,4 +145,11 @@ config PCIE_IPROC_BCMA
   Say Y here if you want to use the Broadcom iProc PCIe controller
   through the BCMA bus interface


snip

 +
 +/* Address translation table entry size */
 +#define ATT_ENTRY_SIZE 8
 +
 +#define DWORD_MASK 3
 +
 +struct altera_pcie {
 +   struct platform_device  *pdev;
 +   struct resource *txs;

You have Txs documented in the bindings document, you have a pointer
here, but you've never used it
anywhre in the code? What is it for?

 +   void __iomem*cra_base;
 +   int irq;
 +   u8  root_bus_nr;
 +   struct irq_domain   *irq_domain;
 +   struct resource bus_range;
 +   struct list_headresources;
 +};
 +

snip

 +
 +static int tlp_cfg_dword_read(struct altera_pcie *pcie, u8 bus, u32 devfn,
 + int where, u32 *value)
 +{
 +   int ret;
 +   u32 headers[TLP_HDR_SIZE];
 +
 +   if (bus == pcie-root_bus_nr)
 +   headers[0] = TLP_CFG_DW0(TLP_FMTTYPE_CFGRD0);
 +   else
 +   headers[0] = TLP_CFG_DW0(TLP_FMTTYPE_CFGRD1);
 +
 +   headers[1] = TLP_CFG_DW1(TLP_REQ_ID(pcie-root_bus_nr, devfn),
 +   TLP_READ_TAG);
 +   headers[2] = TLP_CFG_DW2(bus, devfn, where);
 +
 +   tlp_write_packet(pcie, headers, 0);
 +
 +   ret = tlp_read_packet(pcie, value);
 +   if (ret)
 +   *value = ~0UL;  /* return 0x if error */
 +
 +   return ret;
 +}
 +
 +static int tlp_cfg_dword_write(struct altera_pcie *pcie, u8 bus, u32 devfn,
 +  int where, u32 value)
 +{
 +   u32 headers[TLP_HDR_SIZE];
 +
 +   if (bus == pcie-root_bus_nr)
 +   headers[0] = TLP_CFG_DW0(TLP_FMTTYPE_CFGWR0);
 +   else
 +   headers[0] = TLP_CFG_DW0(TLP_FMTTYPE_CFGWR1);
 +
 +   headers[1] = TLP_CFG_DW1(TLP_REQ_ID(pcie-root_bus_nr, devfn),
 +   TLP_WRITE_TAG);
 +   headers[2] = TLP_CFG_DW2(bus, devfn, where);
 +
 +   tlp_write_packet(pcie, headers, value);
 +
 +   tlp_read_packet(pcie, NULL);

You need to check for the error here.

 +
 +   /* Keep an eye out for changes to the root bus number */
 +   if ((bus == pcie-root_bus_nr)  (where == PCI_PRIMARY_BUS))
 +   pcie-root_bus_nr = (u8)(value);
 +
 +   return PCIBIOS_SUCCESSFUL;
 +}
 +

snip

 +
 +static int altera_pcie_parse_dt(struct altera_pcie *pcie)
 +{
 +   struct resource *cra;
 +   struct platform_device *pdev = pcie-pdev;
 +
 +   cra = platform_get_resource_byname(pdev, IORESOURCE_MEM, Cra);
 +   if (!cra) {
 +   cra = platform_get_resource_byname(pdev, IORESOURCE_MEM, 
 cra);
 +   if (!cra) {
 +   dev_err(pdev-dev,
 +   no cra memory resource defined\n);
 +   return -ENODEV;
 +   }
 +   }
 +

What about Txs?

Dinh
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCHv2] arm64: dts: Add base stratix 10 dtsi

[Dinh Nguyen]

On Tue, 11 Aug 2015, dingu...@opensource.altera.com wrote:

 From: Dinh Nguyen dingu...@opensource.altera.com
 
 Add the base DTS for Altera's SoCFPGA Stratix 10 platform.
 
 Signed-off-by: Dinh Nguyen dingu...@opensource.altera.com
 ---
 v2: use interrupt-affinity for pmu node
 ---
  arch/arm64/Kconfig |   5 +
  arch/arm64/boot/dts/Makefile   |   1 +
  arch/arm64/boot/dts/altera/Makefile|   5 +
  arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi  | 358 
 +
  .../boot/dts/altera/socfpga_stratix10_socdk.dts|  38 +++
  arch/arm64/configs/defconfig   |   1 +
  6 files changed, 408 insertions(+)
  create mode 100644 arch/arm64/boot/dts/altera/Makefile
  create mode 100644 arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
  create mode 100644 arch/arm64/boot/dts/altera/socfpga_stratix10_socdk.dts
 

Soft ping on this pach?

BR,
Dinh
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] arm64: dts: Add base stratix 10 dtsi

[Dinh Nguyen]

Hi Jisheng,

On 8/10/15 9:39 PM, Jisheng Zhang wrote:
> On Mon, 10 Aug 2015 16:09:18 -0500
>  wrote:
> 
>> From: Dinh Nguyen 
>>
>> Add the base DTS for Altera's SoCFPGA Stratix 10 platform.
>>
>> Signed-off-by: Dinh Nguyen 
>> ---
>>  arch/arm64/Kconfig |   5 +
>>  arch/arm64/boot/dts/Makefile   |   1 +
>>  arch/arm64/boot/dts/altera/Makefile|   5 +
>>  arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi  | 354 
>> +
>>  .../boot/dts/altera/socfpga_stratix10_socdk.dts|  38 +++
>>  arch/arm64/configs/defconfig   |   1 +
>>  6 files changed, 404 insertions(+)
>>  create mode 100644 arch/arm64/boot/dts/altera/Makefile
>>  create mode 100644 arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
>>  create mode 100644 arch/arm64/boot/dts/altera/socfpga_stratix10_socdk.dts
>>
>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>> index 318175f..0f8ab2b 100644
>> --- a/arch/arm64/Kconfig
>> +++ b/arch/arm64/Kconfig
>> @@ -207,6 +207,11 @@ config ARCH_SEATTLE
>>  help
>>This enables support for AMD Seattle SOC Family
>>  
>> +config ARCH_STRATIX10
>> +bool "Altera's Stratix 10 SoCFPGA Family"
>> +help
>> +  This enables support for Altera's Stratix 10 SoCFPGA Family
>> +
>>  config ARCH_TEGRA
>>  bool "NVIDIA Tegra SoC Family"
>>  select ARCH_HAS_RESET_CONTROLLER
>> diff --git a/arch/arm64/boot/dts/Makefile b/arch/arm64/boot/dts/Makefile
>> index 38913be..7fb421a 100644
>> --- a/arch/arm64/boot/dts/Makefile
>> +++ b/arch/arm64/boot/dts/Makefile
>> @@ -1,3 +1,4 @@
>> +dts-dirs += altera
>>  dts-dirs += amd
>>  dts-dirs += apm
>>  dts-dirs += arm
>> diff --git a/arch/arm64/boot/dts/altera/Makefile 
>> b/arch/arm64/boot/dts/altera/Makefile
>> new file mode 100644
>> index 000..d7a6416
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/altera/Makefile
>> @@ -0,0 +1,5 @@
>> +dtb-$(CONFIG_ARCH_STRATIX10) += socfpga_stratix10_socdk.dtb
>> +
>> +always  := $(dtb-y)
>> +subdir-y:= $(dts-dirs)
>> +clean-files := *.dtb
>> diff --git a/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi 
>> b/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
>> new file mode 100644
>> index 000..34f6dc3
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
>> @@ -0,0 +1,354 @@
>> +/*
>> + * Copyright Altera Corporation (C) 2015. All rights reserved.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms and conditions of the GNU General Public License,
>> + * version 2, as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope 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/>.
>> + */
>> +
>> +/dts-v1/;
>> +
>> +/ {
>> +compatible = "altr,socfpga-stratix10";
>> +#address-cells = <1>;
>> +#size-cells = <1>;
>> +
> [...]
>> +
>> +pmu {
>> +compatible = "arm,armv8-pmuv3";
>> +interrupts = <0 120 8>,
>> + <0 121 8>,
>> + <0 122 8>,
>> + <0 123 8>;
> 
> it's better to add interrupt-affinity according to Sudeep's suggestions.
> 

Ok, will update in V2. Thanks for reviewing.

Dinh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] arm64: dts: Add base stratix 10 dtsi

[Dinh Nguyen]

Hi Jisheng,

On 8/10/15 9:39 PM, Jisheng Zhang wrote:
 On Mon, 10 Aug 2015 16:09:18 -0500
 dingu...@opensource.altera.com wrote:
 
 From: Dinh Nguyen dingu...@opensource.altera.com

 Add the base DTS for Altera's SoCFPGA Stratix 10 platform.

 Signed-off-by: Dinh Nguyen dingu...@opensource.altera.com
 ---
  arch/arm64/Kconfig |   5 +
  arch/arm64/boot/dts/Makefile   |   1 +
  arch/arm64/boot/dts/altera/Makefile|   5 +
  arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi  | 354 
 +
  .../boot/dts/altera/socfpga_stratix10_socdk.dts|  38 +++
  arch/arm64/configs/defconfig   |   1 +
  6 files changed, 404 insertions(+)
  create mode 100644 arch/arm64/boot/dts/altera/Makefile
  create mode 100644 arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
  create mode 100644 arch/arm64/boot/dts/altera/socfpga_stratix10_socdk.dts

 diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
 index 318175f..0f8ab2b 100644
 --- a/arch/arm64/Kconfig
 +++ b/arch/arm64/Kconfig
 @@ -207,6 +207,11 @@ config ARCH_SEATTLE
  help
This enables support for AMD Seattle SOC Family
  
 +config ARCH_STRATIX10
 +bool Altera's Stratix 10 SoCFPGA Family
 +help
 +  This enables support for Altera's Stratix 10 SoCFPGA Family
 +
  config ARCH_TEGRA
  bool NVIDIA Tegra SoC Family
  select ARCH_HAS_RESET_CONTROLLER
 diff --git a/arch/arm64/boot/dts/Makefile b/arch/arm64/boot/dts/Makefile
 index 38913be..7fb421a 100644
 --- a/arch/arm64/boot/dts/Makefile
 +++ b/arch/arm64/boot/dts/Makefile
 @@ -1,3 +1,4 @@
 +dts-dirs += altera
  dts-dirs += amd
  dts-dirs += apm
  dts-dirs += arm
 diff --git a/arch/arm64/boot/dts/altera/Makefile 
 b/arch/arm64/boot/dts/altera/Makefile
 new file mode 100644
 index 000..d7a6416
 --- /dev/null
 +++ b/arch/arm64/boot/dts/altera/Makefile
 @@ -0,0 +1,5 @@
 +dtb-$(CONFIG_ARCH_STRATIX10) += socfpga_stratix10_socdk.dtb
 +
 +always  := $(dtb-y)
 +subdir-y:= $(dts-dirs)
 +clean-files := *.dtb
 diff --git a/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi 
 b/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
 new file mode 100644
 index 000..34f6dc3
 --- /dev/null
 +++ b/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
 @@ -0,0 +1,354 @@
 +/*
 + * Copyright Altera Corporation (C) 2015. All rights reserved.
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms and conditions of the GNU General Public License,
 + * version 2, as published by the Free Software Foundation.
 + *
 + * This program is distributed in the hope 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/.
 + */
 +
 +/dts-v1/;
 +
 +/ {
 +compatible = altr,socfpga-stratix10;
 +#address-cells = 1;
 +#size-cells = 1;
 +
 [...]
 +
 +pmu {
 +compatible = arm,armv8-pmuv3;
 +interrupts = 0 120 8,
 + 0 121 8,
 + 0 122 8,
 + 0 123 8;
 
 it's better to add interrupt-affinity according to Sudeep's suggestions.
 

Ok, will update in V2. Thanks for reviewing.

Dinh
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 4/6] pci: altera: Add Altera PCIe MSI driver

[Dinh Nguyen]

On Tue, Jul 28, 2015 at 10:07 PM, Ley Foon Tan  wrote:
> On Wed, Jul 29, 2015 at 1:00 AM, Dinh Nguyen  wrote:
>> On Tue, Jul 28, 2015 at 5:45 AM, Ley Foon Tan  wrote:
>>> This patch adds Altera PCIe MSI driver. This soft IP supports configurable
>>> number of vectors, which is a dts parameter.
>>> @@ -154,4 +154,11 @@ config PCIE_ALTERA
>>>   Say Y here if you want to enable PCIe controller support for 
>>> Altera
>>>   SoCFPGA family of SoCs.
>>>
>>> +config PCIE_ALTERA_MSI
>>> +   bool "Altera PCIe MSI feature"
>>> +   depends on PCI_MSI && PCIE_ALTERA
>>> +   help
>>> + Say Y here if you want PCIe MSI support for the Altera SocFPGA 
>>> SoC.
>>> + This MSI driver supports Altera MSI to GIC controller IP.
>>> +
>>
>> Couldn't this driver get combined with the pcie-altera driver?
>
> This MSI IP is a soft IP and it is optional to PCI system. So, we have
> individual driver for it and user can disable it.
>

So why can't you use CONFIG_PCI_MSI in the pcie-altera driver? I see
that most, if not
all of the other drivers are combined with MSI.

Dinh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 4/6] pci: altera: Add Altera PCIe MSI driver

[Dinh Nguyen]

On Tue, Jul 28, 2015 at 5:45 AM, Ley Foon Tan  wrote:
> This patch adds Altera PCIe MSI driver. This soft IP supports configurable
> number of vectors, which is a dts parameter.
>
> Signed-off-by: Ley Foon Tan 
> ---
>  drivers/pci/host/Kconfig   |   7 +
>  drivers/pci/host/Makefile  |   1 +
>  drivers/pci/host/pcie-altera-msi.c | 318 
> +
>  3 files changed, 326 insertions(+)
>  create mode 100644 drivers/pci/host/pcie-altera-msi.c
>
> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
> index af19039..a8b87fd 100644
> --- a/drivers/pci/host/Kconfig
> +++ b/drivers/pci/host/Kconfig
> @@ -154,4 +154,11 @@ config PCIE_ALTERA
>   Say Y here if you want to enable PCIe controller support for Altera
>   SoCFPGA family of SoCs.
>
> +config PCIE_ALTERA_MSI
> +   bool "Altera PCIe MSI feature"
> +   depends on PCI_MSI && PCIE_ALTERA
> +   help
> + Say Y here if you want PCIe MSI support for the Altera SocFPGA SoC.
> + This MSI driver supports Altera MSI to GIC controller IP.
> +

Couldn't this driver get combined with the pcie-altera driver?

Dinh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 3/6] pci:host: Add Altera PCIe host controller driver

[Dinh Nguyen]

On Tue, Jul 28, 2015 at 5:45 AM, Ley Foon Tan  wrote:
> This patch adds the Altera PCIe host controller driver.
>
> Signed-off-by: Ley Foon Tan 
> ---
>  drivers/pci/host/Kconfig   |   9 +
>  drivers/pci/host/Makefile  |   1 +
>  drivers/pci/host/pcie-altera.c | 576 
> +
>  3 files changed, 586 insertions(+)
>  create mode 100644 drivers/pci/host/pcie-altera.c
>
> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
> index 675c2d1..af19039 100644
> --- a/drivers/pci/host/Kconfig
> +++ b/drivers/pci/host/Kconfig
> @@ -145,4 +145,13 @@ config PCIE_IPROC_BCMA
>   Say Y here if you want to use the Broadcom iProc PCIe controller
>   through the BCMA bus interface
>
> +config PCIE_ALTERA
> +   bool "Altera PCIe controller"
> +   depends on ARCH_SOCFPGA
> +   depends on OF

I don't think you need this, "depends on ARCH_SOCFPGA" should already
have taken care of this.

> +   select PCI_MSI_IRQ_DOMAIN if PCI_MSI
> +   help
> + Say Y here if you want to enable PCIe controller support for Altera
> + SoCFPGA family of SoCs.
> +
>  endmenu



> +
> +static int tlp_read_packet(struct altera_pcie *pcie, u32 *value)
> +{
> +   u8 loop;
> +   struct tlp_rp_regpair_t tlp_rp_regdata;
> +
> +   for (loop = TLP_LOOP; loop > 0; loop--) {
> +
> +   tlp_read_rx(pcie, _rp_regdata);
> +
> +   if (tlp_rp_regdata.ctrl & RP_RXCPL_EOP) {
> +
> +   if (value)
> +   *value = tlp_rp_regdata.reg0;
> +
> +   return PCIBIOS_SUCCESSFUL;
> +   }

Remove all the unnecessary newlines in this function.
> +   }
> +
> +   return -ENOENT;
> +}
> +



> +
> +static struct platform_driver altera_pcie_driver = {
> +   .probe  = altera_pcie_probe,
> +   .remove = altera_pcie_remove,
> +   .driver = {
> +   .name   = "altera-pcie",
> +   .owner  = THIS_MODULE,

Don't need to set owner anymore as this will get populated by the driver core.

Dinh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 3/4] reset: socfpga: Update reset-socfpga to read the altr,modrst-offset property

[Dinh Nguyen]

On 7/28/15 3:46 AM, Philipp Zabel wrote:
> Am Montag, den 27.07.2015, 13:57 -0500 schrieb
> dingu...@opensource.altera.com:
>> From: Dinh Nguyen 
>>
>> In order for the Arria10 to be able to re-use the reset driver for SoCFPGA
>> Cyclone5/Arria5, we need to read the 'altr,modrst-offset' property from the
>> device tree entry. The 'altr,modrst-offset' property is the first register
>> into the reset manager that is used for bringing peripherals out of reset.
>>
>> Signed-off-by: Dinh Nguyen 
>> ---
>>  drivers/reset/reset-socfpga.c | 19 +--
>>  1 file changed, 13 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/reset/reset-socfpga.c b/drivers/reset/reset-socfpga.c
>> index 0a8def3..9074d41 100644
>> --- a/drivers/reset/reset-socfpga.c
>> +++ b/drivers/reset/reset-socfpga.c
>> @@ -24,11 +24,11 @@
>>  #include 
>>  
>>  #define NR_BANKS4
>> -#define OFFSET_MODRST   0x10
>>  
>>  struct socfpga_reset_data {
>>  spinlock_t  lock;
>>  void __iomem*membase;
>> +u32 modrst_offset;
>>  struct reset_controller_dev rcdev;
>>  };
>>  
>> @@ -45,8 +45,8 @@ static int socfpga_reset_assert(struct 
>> reset_controller_dev *rcdev,
>>  
>>  spin_lock_irqsave(>lock, flags);
>>  
>> -reg = readl(data->membase + OFFSET_MODRST + (bank * NR_BANKS));
>> -writel(reg | BIT(offset), data->membase + OFFSET_MODRST +
>> +reg = readl(data->membase + data->modrst_offset + (bank * NR_BANKS));
>> +writel(reg | BIT(offset), data->membase + data->modrst_offset +
>>   (bank * NR_BANKS));
>>  spin_unlock_irqrestore(>lock, flags);
>>  
>> @@ -67,8 +67,8 @@ static int socfpga_reset_deassert(struct 
>> reset_controller_dev *rcdev,
>>  
>>  spin_lock_irqsave(>lock, flags);
>>  
>> -reg = readl(data->membase + OFFSET_MODRST + (bank * NR_BANKS));
>> -writel(reg & ~BIT(offset), data->membase + OFFSET_MODRST +
>> +reg = readl(data->membase + data->modrst_offset + (bank * NR_BANKS));
>> +writel(reg & ~BIT(offset), data->membase + data->modrst_offset +
>>(bank * NR_BANKS));
>>  
>>  spin_unlock_irqrestore(>lock, flags);
>> @@ -85,7 +85,7 @@ static int socfpga_reset_status(struct 
>> reset_controller_dev *rcdev,
>>  int offset = id % BITS_PER_LONG;
>>  u32 reg;
>>  
>> -reg = readl(data->membase + OFFSET_MODRST + (bank * NR_BANKS));
>> +reg = readl(data->membase + data->modrst_offset + (bank * NR_BANKS));
>>  
>>  return !(reg & BIT(offset));
>>  }
>> @@ -100,6 +100,8 @@ static int socfpga_reset_probe(struct platform_device 
>> *pdev)
>>  {
>>  struct socfpga_reset_data *data;
>>  struct resource *res;
>> +struct device *dev = >dev;
>> +struct device_node *np = dev->of_node;
>>  
>>  /*
>>   * The binding was mainlined without the required property.
>> @@ -120,6 +122,11 @@ static int socfpga_reset_probe(struct platform_device 
>> *pdev)
>>  if (IS_ERR(data->membase))
>>  return PTR_ERR(data->membase);
>>  
>> +if (of_property_read_u32(np, "altr,modrst-offset", 
>> >modrst_offset)) {
>> +dev_err(dev, "no altr,modrst-offset specified in device 
>> tree\n");
>> +return -ENODEV;
>> +}
>> +
> 
> This should fall back to the old value of 0x10 in case the device tree
> property doesn't exist. Otherwise you are breaking Cyclone5/Arria5 with
> older device trees.
> 

Ah yes, you're right. Thanks for catching this!

Dinh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 3/4] reset: socfpga: Update reset-socfpga to read the altr,modrst-offset property

[Dinh Nguyen]

On 7/28/15 3:46 AM, Philipp Zabel wrote:
 Am Montag, den 27.07.2015, 13:57 -0500 schrieb
 dingu...@opensource.altera.com:
 From: Dinh Nguyen dingu...@opensource.altera.com

 In order for the Arria10 to be able to re-use the reset driver for SoCFPGA
 Cyclone5/Arria5, we need to read the 'altr,modrst-offset' property from the
 device tree entry. The 'altr,modrst-offset' property is the first register
 into the reset manager that is used for bringing peripherals out of reset.

 Signed-off-by: Dinh Nguyen dingu...@opensource.altera.com
 ---
  drivers/reset/reset-socfpga.c | 19 +--
  1 file changed, 13 insertions(+), 6 deletions(-)

 diff --git a/drivers/reset/reset-socfpga.c b/drivers/reset/reset-socfpga.c
 index 0a8def3..9074d41 100644
 --- a/drivers/reset/reset-socfpga.c
 +++ b/drivers/reset/reset-socfpga.c
 @@ -24,11 +24,11 @@
  #include linux/types.h
  
  #define NR_BANKS4
 -#define OFFSET_MODRST   0x10
  
  struct socfpga_reset_data {
  spinlock_t  lock;
  void __iomem*membase;
 +u32 modrst_offset;
  struct reset_controller_dev rcdev;
  };
  
 @@ -45,8 +45,8 @@ static int socfpga_reset_assert(struct 
 reset_controller_dev *rcdev,
  
  spin_lock_irqsave(data-lock, flags);
  
 -reg = readl(data-membase + OFFSET_MODRST + (bank * NR_BANKS));
 -writel(reg | BIT(offset), data-membase + OFFSET_MODRST +
 +reg = readl(data-membase + data-modrst_offset + (bank * NR_BANKS));
 +writel(reg | BIT(offset), data-membase + data-modrst_offset +
   (bank * NR_BANKS));
  spin_unlock_irqrestore(data-lock, flags);
  
 @@ -67,8 +67,8 @@ static int socfpga_reset_deassert(struct 
 reset_controller_dev *rcdev,
  
  spin_lock_irqsave(data-lock, flags);
  
 -reg = readl(data-membase + OFFSET_MODRST + (bank * NR_BANKS));
 -writel(reg  ~BIT(offset), data-membase + OFFSET_MODRST +
 +reg = readl(data-membase + data-modrst_offset + (bank * NR_BANKS));
 +writel(reg  ~BIT(offset), data-membase + data-modrst_offset +
(bank * NR_BANKS));
  
  spin_unlock_irqrestore(data-lock, flags);
 @@ -85,7 +85,7 @@ static int socfpga_reset_status(struct 
 reset_controller_dev *rcdev,
  int offset = id % BITS_PER_LONG;
  u32 reg;
  
 -reg = readl(data-membase + OFFSET_MODRST + (bank * NR_BANKS));
 +reg = readl(data-membase + data-modrst_offset + (bank * NR_BANKS));
  
  return !(reg  BIT(offset));
  }
 @@ -100,6 +100,8 @@ static int socfpga_reset_probe(struct platform_device 
 *pdev)
  {
  struct socfpga_reset_data *data;
  struct resource *res;
 +struct device *dev = pdev-dev;
 +struct device_node *np = dev-of_node;
  
  /*
   * The binding was mainlined without the required property.
 @@ -120,6 +122,11 @@ static int socfpga_reset_probe(struct platform_device 
 *pdev)
  if (IS_ERR(data-membase))
  return PTR_ERR(data-membase);
  
 +if (of_property_read_u32(np, altr,modrst-offset, 
 data-modrst_offset)) {
 +dev_err(dev, no altr,modrst-offset specified in device 
 tree\n);
 +return -ENODEV;
 +}
 +
 
 This should fall back to the old value of 0x10 in case the device tree
 property doesn't exist. Otherwise you are breaking Cyclone5/Arria5 with
 older device trees.
 

Ah yes, you're right. Thanks for catching this!

Dinh
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 4/6] pci: altera: Add Altera PCIe MSI driver

[Dinh Nguyen]

On Tue, Jul 28, 2015 at 10:07 PM, Ley Foon Tan lf...@altera.com wrote:
 On Wed, Jul 29, 2015 at 1:00 AM, Dinh Nguyen dinh.li...@gmail.com wrote:
 On Tue, Jul 28, 2015 at 5:45 AM, Ley Foon Tan lf...@altera.com wrote:
 This patch adds Altera PCIe MSI driver. This soft IP supports configurable
 number of vectors, which is a dts parameter.
 @@ -154,4 +154,11 @@ config PCIE_ALTERA
   Say Y here if you want to enable PCIe controller support for 
 Altera
   SoCFPGA family of SoCs.

 +config PCIE_ALTERA_MSI
 +   bool Altera PCIe MSI feature
 +   depends on PCI_MSI  PCIE_ALTERA
 +   help
 + Say Y here if you want PCIe MSI support for the Altera SocFPGA 
 SoC.
 + This MSI driver supports Altera MSI to GIC controller IP.
 +

 Couldn't this driver get combined with the pcie-altera driver?

 This MSI IP is a soft IP and it is optional to PCI system. So, we have
 individual driver for it and user can disable it.


So why can't you use CONFIG_PCI_MSI in the pcie-altera driver? I see
that most, if not
all of the other drivers are combined with MSI.

Dinh
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 3/6] pci:host: Add Altera PCIe host controller driver

[Dinh Nguyen]

On Tue, Jul 28, 2015 at 5:45 AM, Ley Foon Tan lf...@altera.com wrote:
 This patch adds the Altera PCIe host controller driver.

 Signed-off-by: Ley Foon Tan lf...@altera.com
 ---
  drivers/pci/host/Kconfig   |   9 +
  drivers/pci/host/Makefile  |   1 +
  drivers/pci/host/pcie-altera.c | 576 
 +
  3 files changed, 586 insertions(+)
  create mode 100644 drivers/pci/host/pcie-altera.c

 diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
 index 675c2d1..af19039 100644
 --- a/drivers/pci/host/Kconfig
 +++ b/drivers/pci/host/Kconfig
 @@ -145,4 +145,13 @@ config PCIE_IPROC_BCMA
   Say Y here if you want to use the Broadcom iProc PCIe controller
   through the BCMA bus interface

 +config PCIE_ALTERA
 +   bool Altera PCIe controller
 +   depends on ARCH_SOCFPGA
 +   depends on OF

I don't think you need this, depends on ARCH_SOCFPGA should already
have taken care of this.

 +   select PCI_MSI_IRQ_DOMAIN if PCI_MSI
 +   help
 + Say Y here if you want to enable PCIe controller support for Altera
 + SoCFPGA family of SoCs.
 +
  endmenu

snip

 +
 +static int tlp_read_packet(struct altera_pcie *pcie, u32 *value)
 +{
 +   u8 loop;
 +   struct tlp_rp_regpair_t tlp_rp_regdata;
 +
 +   for (loop = TLP_LOOP; loop  0; loop--) {
 +
 +   tlp_read_rx(pcie, tlp_rp_regdata);
 +
 +   if (tlp_rp_regdata.ctrl  RP_RXCPL_EOP) {
 +
 +   if (value)
 +   *value = tlp_rp_regdata.reg0;
 +
 +   return PCIBIOS_SUCCESSFUL;
 +   }

Remove all the unnecessary newlines in this function.
 +   }
 +
 +   return -ENOENT;
 +}
 +

snip

 +
 +static struct platform_driver altera_pcie_driver = {
 +   .probe  = altera_pcie_probe,
 +   .remove = altera_pcie_remove,
 +   .driver = {
 +   .name   = altera-pcie,
 +   .owner  = THIS_MODULE,

Don't need to set owner anymore as this will get populated by the driver core.

Dinh
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 4/6] pci: altera: Add Altera PCIe MSI driver

[Dinh Nguyen]

On Tue, Jul 28, 2015 at 5:45 AM, Ley Foon Tan lf...@altera.com wrote:
 This patch adds Altera PCIe MSI driver. This soft IP supports configurable
 number of vectors, which is a dts parameter.

 Signed-off-by: Ley Foon Tan lf...@altera.com
 ---
  drivers/pci/host/Kconfig   |   7 +
  drivers/pci/host/Makefile  |   1 +
  drivers/pci/host/pcie-altera-msi.c | 318 
 +
  3 files changed, 326 insertions(+)
  create mode 100644 drivers/pci/host/pcie-altera-msi.c

 diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
 index af19039..a8b87fd 100644
 --- a/drivers/pci/host/Kconfig
 +++ b/drivers/pci/host/Kconfig
 @@ -154,4 +154,11 @@ config PCIE_ALTERA
   Say Y here if you want to enable PCIe controller support for Altera
   SoCFPGA family of SoCs.

 +config PCIE_ALTERA_MSI
 +   bool Altera PCIe MSI feature
 +   depends on PCI_MSI  PCIE_ALTERA
 +   help
 + Say Y here if you want PCIe MSI support for the Altera SocFPGA SoC.
 + This MSI driver supports Altera MSI to GIC controller IP.
 +

Couldn't this driver get combined with the pcie-altera driver?

Dinh
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] clk: socfpga: Add a second parent option for the dbg_base_clk

[Dinh Nguyen]

On 7/24/15 4:41 PM, Stephen Boyd wrote:
> On 07/22, dingu...@opensource.altera.com wrote:
>> diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi
>> index 80f924d..7d5db54 100644
>> --- a/arch/arm/boot/dts/socfpga.dtsi
>> +++ b/arch/arm/boot/dts/socfpga.dtsi
>> @@ -164,7 +164,7 @@
>>  dbg_base_clk: dbg_base_clk {
>>  #clock-cells = <0>;
>>  compatible = 
>> "altr,socfpga-perip-clk";
>> -clocks = <_pll>;
>> +clocks = <_pll>, 
>> <>;
>>  div-reg = <0xe8 0 9>;
>>  reg = <0x50>;
>>  };
> 
> We don't usually take changes in dts files. Can you split this
> off and take it through arm-soc?
> 

Ah okay..will do.

Thanks,
Dinh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] clk: socfpga: Add a second parent option for the dbg_base_clk

[Dinh Nguyen]

On 7/24/15 4:41 PM, Stephen Boyd wrote:
 On 07/22, dingu...@opensource.altera.com wrote:
 diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi
 index 80f924d..7d5db54 100644
 --- a/arch/arm/boot/dts/socfpga.dtsi
 +++ b/arch/arm/boot/dts/socfpga.dtsi
 @@ -164,7 +164,7 @@
  dbg_base_clk: dbg_base_clk {
  #clock-cells = 0;
  compatible = 
 altr,socfpga-perip-clk;
 -clocks = main_pll;
 +clocks = main_pll, 
 osc1;
  div-reg = 0xe8 0 9;
  reg = 0x50;
  };
 
 We don't usually take changes in dts files. Can you split this
 off and take it through arm-soc?
 

Ah okay..will do.

Thanks,
Dinh
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCHv2 5/6] clk: sunxi: make use of of_clk_parent_fill helper function

[Dinh Nguyen]

On Tue, 21 Jul 2015, Tyler Baker wrote:

> Hi,
> 
> On 17 July 2015 at 17:53, Stephen Boyd  wrote:
> > On 07/06, dingu...@opensource.altera.com wrote:
> >> From: Dinh Nguyen 
> >>
> >> Use of_clk_parent_fill to fill in the parent clock names' array.
> >>
> >> Signed-off-by: Dinh Nguyen 
> >> Cc: Maxime Ripard 
> >> Cc: "Emilio López" 
> >> ---
> >
> > Applied to clk-next with that fix I posted.
> 
> The kernelci.org bot reported sunxi a20 boot failures[1][2] in
> next-20150720 and next-20150721. I have bisected[3] these failures to
> this commit. However, I have not investigated any further but
> reverting this commit on top of next-20150721 gets the boards booting
> again.
>

Can you please try this patch?

-->8-

diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
index e8c5185..958655f 100644
--- a/drivers/clk/sunxi/clk-sunxi.c
+++ b/drivers/clk/sunxi/clk-sunxi.c
@@ -204,7 +204,7 @@ static void __init sun6i_ahb1_clk_setup(struct device_node 
*node)
return;
 
/* we have a mux, we will have >1 parents */
-   of_clk_parent_fill(node, parents, SUN6I_AHB1_MAX_PARENTS);
+   i = of_clk_parent_fill(node, parents, SUN6I_AHB1_MAX_PARENTS);
of_property_read_string(node, "clock-output-names", _name);
 
ahb1 = kzalloc(sizeof(struct sun6i_ahb1_clk), GFP_KERNEL);
@@ -789,7 +789,7 @@ static void __init sunxi_mux_clk_setup(struct device_node 
*node,
 
reg = of_iomap(node, 0);
 
-   of_clk_parent_fill(node, parents, SUNXI_MAX_PARENTS);
+   i = of_clk_parent_fill(node, parents, SUNXI_MAX_PARENTS);
of_property_read_string(node, "clock-output-names", _name);
 
clk = clk_register_mux(NULL, clk_name, parents, i,

Re: [PATCHv2 5/6] clk: sunxi: make use of of_clk_parent_fill helper function

[Dinh Nguyen]

On Tue, 21 Jul 2015, Tyler Baker wrote:

 Hi,
 
 On 17 July 2015 at 17:53, Stephen Boyd sb...@codeaurora.org wrote:
  On 07/06, dingu...@opensource.altera.com wrote:
  From: Dinh Nguyen dingu...@opensource.altera.com
 
  Use of_clk_parent_fill to fill in the parent clock names' array.
 
  Signed-off-by: Dinh Nguyen dingu...@opensource.altera.com
  Cc: Maxime Ripard maxime.rip...@free-electrons.com
  Cc: Emilio López emi...@elopez.com.ar
  ---
 
  Applied to clk-next with that fix I posted.
 
 The kernelci.org bot reported sunxi a20 boot failures[1][2] in
 next-20150720 and next-20150721. I have bisected[3] these failures to
 this commit. However, I have not investigated any further but
 reverting this commit on top of next-20150721 gets the boards booting
 again.


Can you please try this patch?

--8-

diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
index e8c5185..958655f 100644
--- a/drivers/clk/sunxi/clk-sunxi.c
+++ b/drivers/clk/sunxi/clk-sunxi.c
@@ -204,7 +204,7 @@ static void __init sun6i_ahb1_clk_setup(struct device_node 
*node)
return;
 
/* we have a mux, we will have 1 parents */
-   of_clk_parent_fill(node, parents, SUN6I_AHB1_MAX_PARENTS);
+   i = of_clk_parent_fill(node, parents, SUN6I_AHB1_MAX_PARENTS);
of_property_read_string(node, clock-output-names, clk_name);
 
ahb1 = kzalloc(sizeof(struct sun6i_ahb1_clk), GFP_KERNEL);
@@ -789,7 +789,7 @@ static void __init sunxi_mux_clk_setup(struct device_node 
*node,
 
reg = of_iomap(node, 0);
 
-   of_clk_parent_fill(node, parents, SUNXI_MAX_PARENTS);
+   i = of_clk_parent_fill(node, parents, SUNXI_MAX_PARENTS);
of_property_read_string(node, clock-output-names, clk_name);
 
clk = clk_register_mux(NULL, clk_name, parents, i,

Re: [PATCH 24/45] clk: socfpga: Remove clk.h and clkdev.h includes

[Dinh Nguyen]

On 07/10/2015 06:33 PM, Stephen Boyd wrote:
> Clock provider drivers generally shouldn't include clk.h because
> it's the consumer API. Remove the include here because this is a
> provider driver. The clkdev.h include isn't used either, remove
> it and add in slab.h to make sure things keep compiling.
> 
> Cc: Dinh Nguyen 
> Signed-off-by: Stephen Boyd 
> ---
>  drivers/clk/socfpga/clk-gate-a10.c   | 1 +
>  drivers/clk/socfpga/clk-gate.c   | 3 +--
>  drivers/clk/socfpga/clk-periph-a10.c | 1 +
>  drivers/clk/socfpga/clk-periph.c | 3 +--
>  drivers/clk/socfpga/clk-pll-a10.c| 1 +
>  drivers/clk/socfpga/clk-pll.c| 3 +--
>  drivers/clk/socfpga/clk.h| 1 -
>  7 files changed, 6 insertions(+), 7 deletions(-)
> 

Acked-by: Dinh Nguyen 

Thanks,
Dinh

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v5 3/8] clk: socfpga: switch to GENMASK()

[Dinh Nguyen]

On 07/13/2015 09:07 AM, Andy Shevchenko wrote:
> Convert the code to use GENMASK() helper instead of div_mask() macro.
> 
> Signed-off-by: Andy Shevchenko 
> ---
>  drivers/clk/socfpga/clk-gate-a10.c   | 2 +-
>  drivers/clk/socfpga/clk-gate.c   | 2 +-
>  drivers/clk/socfpga/clk-periph-a10.c | 2 +-
>  drivers/clk/socfpga/clk-periph.c | 2 +-
>  drivers/clk/socfpga/clk.h| 1 -
>  5 files changed, 4 insertions(+), 5 deletions(-)
> 

Acked-by: Dinh Nguyen 

Thanks,
Dinh

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 24/45] clk: socfpga: Remove clk.h and clkdev.h includes

[Dinh Nguyen]

On 07/10/2015 06:33 PM, Stephen Boyd wrote:
 Clock provider drivers generally shouldn't include clk.h because
 it's the consumer API. Remove the include here because this is a
 provider driver. The clkdev.h include isn't used either, remove
 it and add in slab.h to make sure things keep compiling.
 
 Cc: Dinh Nguyen dingu...@opensource.altera.com
 Signed-off-by: Stephen Boyd sb...@codeaurora.org
 ---
  drivers/clk/socfpga/clk-gate-a10.c   | 1 +
  drivers/clk/socfpga/clk-gate.c   | 3 +--
  drivers/clk/socfpga/clk-periph-a10.c | 1 +
  drivers/clk/socfpga/clk-periph.c | 3 +--
  drivers/clk/socfpga/clk-pll-a10.c| 1 +
  drivers/clk/socfpga/clk-pll.c| 3 +--
  drivers/clk/socfpga/clk.h| 1 -
  7 files changed, 6 insertions(+), 7 deletions(-)
 

Acked-by: Dinh Nguyen dingu...@opensource.altera.com

Thanks,
Dinh

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

Re: [PATCH v5 3/8] clk: socfpga: switch to GENMASK()

[Dinh Nguyen]

On 07/13/2015 09:07 AM, Andy Shevchenko wrote:
 Convert the code to use GENMASK() helper instead of div_mask() macro.
 
 Signed-off-by: Andy Shevchenko andriy.shevche...@linux.intel.com
 ---
  drivers/clk/socfpga/clk-gate-a10.c   | 2 +-
  drivers/clk/socfpga/clk-gate.c   | 2 +-
  drivers/clk/socfpga/clk-periph-a10.c | 2 +-
  drivers/clk/socfpga/clk-periph.c | 2 +-
  drivers/clk/socfpga/clk.h| 1 -
  5 files changed, 4 insertions(+), 5 deletions(-)
 

Acked-by: Dinh Nguyen dingu...@opensource.altera.com

Thanks,
Dinh

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

Re: [PATCH v4 3/6] clk: socfpga: switch to GENMASK()

[Dinh Nguyen]

Hi Andy,

On 07/09/2015 11:43 AM, Andy Shevchenko wrote:
> Convert the code to use GENMASK() helper instead of div_mask() macro.
> 
> Signed-off-by: Andy Shevchenko 
> ---
>  drivers/clk/socfpga/clk-gate.c   | 2 +-
>  drivers/clk/socfpga/clk-periph.c | 2 +-
>  drivers/clk/socfpga/clk.h| 1 -
>  3 files changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/clk/socfpga/clk-gate.c b/drivers/clk/socfpga/clk-gate.c
> index dd3a78c..d61052e 100644
> --- a/drivers/clk/socfpga/clk-gate.c
> +++ b/drivers/clk/socfpga/clk-gate.c
> @@ -110,7 +110,7 @@ static unsigned long socfpga_clk_recalc_rate(struct 
> clk_hw *hwclk,
>   div = socfpgaclk->fixed_div;
>   else if (socfpgaclk->div_reg) {
>   val = readl(socfpgaclk->div_reg) >> socfpgaclk->shift;
> - val &= div_mask(socfpgaclk->width);
> + val &= GENMASK(socfpgaclk->width - 1, 0);
>   /* Check for GPIO_DB_CLK by its offset */
>   if ((int) socfpgaclk->div_reg & SOCFPGA_GPIO_DB_CLK_OFFSET)
>   div = val + 1;
> diff --git a/drivers/clk/socfpga/clk-periph.c 
> b/drivers/clk/socfpga/clk-periph.c
> index 46531c3..fc410a4 100644
> --- a/drivers/clk/socfpga/clk-periph.c
> +++ b/drivers/clk/socfpga/clk-periph.c
> @@ -36,7 +36,7 @@ static unsigned long clk_periclk_recalc_rate(struct clk_hw 
> *hwclk,
>   } else {
>   if (socfpgaclk->div_reg) {
>   val = readl(socfpgaclk->div_reg) >> socfpgaclk->shift;
> - val &= div_mask(socfpgaclk->width);
> + val &= GENMASK(socfpgaclk->width - 1, 0);
>   parent_rate /= (val + 1);
>   }
>   div = ((readl(socfpgaclk->hw.reg) & 0x1ff) + 1);
> diff --git a/drivers/clk/socfpga/clk.h b/drivers/clk/socfpga/clk.h
> index d291f60..5278156 100644
> --- a/drivers/clk/socfpga/clk.h
> +++ b/drivers/clk/socfpga/clk.h
> @@ -27,7 +27,6 @@
>  #define CLKMGR_PERPLL_SRC0xAC
>  
>  #define SOCFPGA_MAX_PARENTS  3
> -#define div_mask(width) ((1 << (width)) - 1)
>  
>  extern void __iomem *clk_mgr_base_addr;
>  
> 

Thanks for doing this, but this patch did not apply for me on v4.2-rc1.
Also, there are now socfpga/clk-gate-a10.c and socfpga/clk-periph-a10.c
that would also need to use GENMASK. Can you please rebase and resend?

Thanks,
Dinh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v4 3/6] clk: socfpga: switch to GENMASK()

[Dinh Nguyen]

Hi Andy,

On 07/09/2015 11:43 AM, Andy Shevchenko wrote:
 Convert the code to use GENMASK() helper instead of div_mask() macro.
 
 Signed-off-by: Andy Shevchenko andriy.shevche...@linux.intel.com
 ---
  drivers/clk/socfpga/clk-gate.c   | 2 +-
  drivers/clk/socfpga/clk-periph.c | 2 +-
  drivers/clk/socfpga/clk.h| 1 -
  3 files changed, 2 insertions(+), 3 deletions(-)
 
 diff --git a/drivers/clk/socfpga/clk-gate.c b/drivers/clk/socfpga/clk-gate.c
 index dd3a78c..d61052e 100644
 --- a/drivers/clk/socfpga/clk-gate.c
 +++ b/drivers/clk/socfpga/clk-gate.c
 @@ -110,7 +110,7 @@ static unsigned long socfpga_clk_recalc_rate(struct 
 clk_hw *hwclk,
   div = socfpgaclk-fixed_div;
   else if (socfpgaclk-div_reg) {
   val = readl(socfpgaclk-div_reg)  socfpgaclk-shift;
 - val = div_mask(socfpgaclk-width);
 + val = GENMASK(socfpgaclk-width - 1, 0);
   /* Check for GPIO_DB_CLK by its offset */
   if ((int) socfpgaclk-div_reg  SOCFPGA_GPIO_DB_CLK_OFFSET)
   div = val + 1;
 diff --git a/drivers/clk/socfpga/clk-periph.c 
 b/drivers/clk/socfpga/clk-periph.c
 index 46531c3..fc410a4 100644
 --- a/drivers/clk/socfpga/clk-periph.c
 +++ b/drivers/clk/socfpga/clk-periph.c
 @@ -36,7 +36,7 @@ static unsigned long clk_periclk_recalc_rate(struct clk_hw 
 *hwclk,
   } else {
   if (socfpgaclk-div_reg) {
   val = readl(socfpgaclk-div_reg)  socfpgaclk-shift;
 - val = div_mask(socfpgaclk-width);
 + val = GENMASK(socfpgaclk-width - 1, 0);
   parent_rate /= (val + 1);
   }
   div = ((readl(socfpgaclk-hw.reg)  0x1ff) + 1);
 diff --git a/drivers/clk/socfpga/clk.h b/drivers/clk/socfpga/clk.h
 index d291f60..5278156 100644
 --- a/drivers/clk/socfpga/clk.h
 +++ b/drivers/clk/socfpga/clk.h
 @@ -27,7 +27,6 @@
  #define CLKMGR_PERPLL_SRC0xAC
  
  #define SOCFPGA_MAX_PARENTS  3
 -#define div_mask(width) ((1  (width)) - 1)
  
  extern void __iomem *clk_mgr_base_addr;
  
 

Thanks for doing this, but this patch did not apply for me on v4.2-rc1.
Also, there are now socfpga/clk-gate-a10.c and socfpga/clk-periph-a10.c
that would also need to use GENMASK. Can you please rebase and resend?

Thanks,
Dinh
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/3] ARM: socfpga: dts: Fix adxl34x formating

[Dinh Nguyen]

On 6/26/15 5:36 AM, Walter Lozano wrote:
> Hi Steffen
> 
> On Fri, Jun 26, 2015 at 4:16 AM, Steffen Trumtrar
>  wrote:
>> Hi Walter!
>>
>> On Thu, Jun 25, 2015 at 11:25:57PM -0300, Walter Lozano wrote:
>>> This patch fixes the formating of DTS bindings for the adxl34x digital
>>> accelerometer.
>>>
>>> Signed-off-by: Walter Lozano 
>>> ---
>>>  arch/arm/boot/dts/socfpga_cyclone5_sockit.dts |6 +++---
>>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/arch/arm/boot/dts/socfpga_cyclone5_sockit.dts 
>>> b/arch/arm/boot/dts/socfpga_cyclone5_sockit.dts
>>> index 71468a7..5b60692 100644
>>> --- a/arch/arm/boot/dts/socfpga_cyclone5_sockit.dts
>>> +++ b/arch/arm/boot/dts/socfpga_cyclone5_sockit.dts
>>> @@ -73,14 +73,14 @@
>>>   status = "okay";
>>>  };
>>>
>>> -{
>>> + {
>>>   status = "okay";
>>>
>>> - accel1: accel1@53{
>>> + accel1: accelerometer@53 {
>>>   compatible = "adxl34x";
>>>   reg = <0x53>;
>>>
>>> - interrupt-parent = <  >;
>>> + interrupt-parent = <>;
>>>   interrupts = <3 2>;
>>>   };
>>>  };
>>
>> I would personally squash 3/3 into this one, but otherwise:
>>
>> Whole series
>>
>> Acked-by: Steffen Trumtrar 
>>
> 
> Thanks for your comments. I have no problem at all. Dinh?

I've applied and squashed 1/3 and 3/3 together.

Thanks,
Dinh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/3] ARM: socfpga: dts: Fix adxl34x formating

[Dinh Nguyen]

On 6/26/15 5:36 AM, Walter Lozano wrote:
 Hi Steffen
 
 On Fri, Jun 26, 2015 at 4:16 AM, Steffen Trumtrar
 s.trumt...@pengutronix.de wrote:
 Hi Walter!

 On Thu, Jun 25, 2015 at 11:25:57PM -0300, Walter Lozano wrote:
 This patch fixes the formating of DTS bindings for the adxl34x digital
 accelerometer.

 Signed-off-by: Walter Lozano wal...@vanguardiasur.com.ar
 ---
  arch/arm/boot/dts/socfpga_cyclone5_sockit.dts |6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

 diff --git a/arch/arm/boot/dts/socfpga_cyclone5_sockit.dts 
 b/arch/arm/boot/dts/socfpga_cyclone5_sockit.dts
 index 71468a7..5b60692 100644
 --- a/arch/arm/boot/dts/socfpga_cyclone5_sockit.dts
 +++ b/arch/arm/boot/dts/socfpga_cyclone5_sockit.dts
 @@ -73,14 +73,14 @@
   status = okay;
  };

 -i2c1{
 +i2c1 {
   status = okay;

 - accel1: accel1@53{
 + accel1: accelerometer@53 {
   compatible = adxl34x;
   reg = 0x53;

 - interrupt-parent =  portc ;
 + interrupt-parent = portc;
   interrupts = 3 2;
   };
  };

 I would personally squash 3/3 into this one, but otherwise:

 Whole series

 Acked-by: Steffen Trumtrar s.trumt...@pengutronix.de

 
 Thanks for your comments. I have no problem at all. Dinh?

I've applied and squashed 1/3 and 3/3 together.

Thanks,
Dinh
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2] ARM: socfpga: dts: Add adxl34x

[Dinh Nguyen]

On 6/25/15 12:01 PM, Walter Lozano wrote:
> On Thu, Jun 25, 2015 at 4:28 AM, Geert Uytterhoeven
>  wrote:
>> On Tue, Jun 23, 2015 at 10:20 PM, Walter Lozano
>>  wrote:
>>> On Mon, Mar 23, 2015 at 11:29 PM, Walter Lozano
>>>  wrote:
>>>> On Mon, Mar 23, 2015 at 12:03 AM, Dinh Nguyen
>>>>  wrote:
>>>>> On 3/19/15 4:27 PM, Walter Lozano wrote:
>>>>>> On Mon, Mar 16, 2015 at 10:10 AM, Walter Lozano
>>>>>>  wrote:
>>>>>>> On Mon, Jan 5, 2015 at 6:21 AM, Steffen Trumtrar
>>>>>>>  wrote:
>>>>>>>> On Wed, Dec 24, 2014 at 08:11:52PM -0300, Walter Lozano wrote:
>>>>>>>>> This patch adds the DTS bindings for the adxl34x digital
>>>>>>>>> accelerometer.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Walter Lozano 
>>>>>>>>> ---
>>>>>>>>>  arch/arm/boot/dts/socfpga_cyclone5_sockit.dts |   16 
>>>>>>>>>  1 file changed, 16 insertions(+)
>>>>>>>>>
>>>>>>>>> diff --git a/arch/arm/boot/dts/socfpga_cyclone5_sockit.dts 
>>>>>>>>> b/arch/arm/boot/dts/socfpga_cyclone5_sockit.dts
>>>>>>>>> index 16ea6f5..d343e03 100644
>>>>>>>>> --- a/arch/arm/boot/dts/socfpga_cyclone5_sockit.dts
>>>>>>>>> +++ b/arch/arm/boot/dts/socfpga_cyclone5_sockit.dts
>>>>>>>>> @@ -60,6 +60,22 @@
>>>>>>>>>   rxc-skew-ps = <2000>;
>>>>>>>>>  };
>>>>>>>>>
>>>>>>>>> + {
>>>>>>>>> + status = "okay";
>>>>>>>>> +};
>>>>>>>>> +
>>>>>>>>> + {
>>>>>>>>> + status = "okay";
>>>>>>>>> +
>>>>>>>>> + accel1: accelerometer@53 {
>>>>>>>>> + compatible = "adxl34x";
>>>>>
>>>>> Shouldn't this be "adi,adxl34x"? At least that's what the documentation
>>>>> says.
>>>>>
>>>>>>>>> + reg = <0x53>;
>>>>>>>>> +
>>>>>>>>> + interrupt-parent = <>;
>>>>>>>>> + interrupts = <3 2>;
>>>>>>>>> + };
>>>>>>>>> +};
>>>>>>>>> +
>>>>>>>>>   {
>>>>>>>>>   vmmc-supply = <_3_3v>;
>>>>>>>>>   vqmmc-supply = <_3_3v>;
>>>>>>>>
>>>>>>>> I just gave it a short spin. I get some interrupts and the position 
>>>>>>>> property
>>>>>>>> changes, so it seems to work:
>>>>>>>>
>>>>>>>> Acked-by: Steffen Trumtrar 
>>>>>>>>
>>>>>>>
>>>>>>> Ping?
>>>>>>> How should this continue?
>>>>>>
>>>>>> Any news about this patch? It has been acked by Pavel Machek and
>>>>>> Steffen Trumtrar a while ago.
>>>>>>
>>>>>
>>>>> Sorry, since I wasn't CC on the original patch and somehow my filter
>>>>> didn't catch it, I didn't see this patch until now.
>>>>
>>>> No problem, I know it was my fault as I trust the output of 
>>>> get_maintainer.pl
>>>>
>>>
>>> Ping?
>>>
>>> --
>>> Walter Lozano, VanguardiaSur
>>> www.vanguardiasur.com.ar
>>> --
>>> 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
>>
>> Now these commits are in:
>>   - 3a38958d2477b718 ("Input: adxl34x - add OF match support") [1]
>>   - e465bf6fc55d5ce2 ("DT: i2c: Deprecate adi,adxl34x compatible string") [2]
>>
>> you can use "adi,adxl345" (or "adi,adxl346", "adi,adxl345").
>>
>> [1] 
>> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=3a38958d2477b718d1011fb88c24c4f264ee9700
>> [2] 
>> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=e465bf6fc55d5ce21fb45a75c3fa613505e6be20
>>
>> Gr{oetje,eeting}s,
>>
>> Geert
>>
>> --
>> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- 
>> ge...@linux-m68k.org
>>
>> In personal conversations with technical people, I call myself a hacker. But
>> when I'm talking to journalists I just say "programmer" or something like 
>> that.
>> -- Linus Torvalds
> 
> 
> Thanks for the information Geert.
> 
> Dihn, as I can see you have apply an old version of the this patch
> (V1), instead of V2 which has some format corrections suggested by
> Steffen Trumtrar. I addition, as Geert points, the compatibility
> string should now be updated.
> 
> How would you like to proceed?
> 

Your commit is staged for v4.2 in the arm-soc tree[1]. Please submit a
patch to correct it for an -rc.

[1]
https://git.kernel.org/cgit/linux/kernel/git/arm/arm-soc.git/commit/arch/arm/boot/dts/socfpga_cyclone5_sockit.dts?h=for-next=bf513bf04660321eb6181082def79ee1fd93b48f

Dinh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2] ARM: socfpga: dts: Add adxl34x

[Dinh Nguyen]

On 6/25/15 12:01 PM, Walter Lozano wrote:
 On Thu, Jun 25, 2015 at 4:28 AM, Geert Uytterhoeven
 ge...@linux-m68k.org wrote:
 On Tue, Jun 23, 2015 at 10:20 PM, Walter Lozano
 wal...@vanguardiasur.com.ar wrote:
 On Mon, Mar 23, 2015 at 11:29 PM, Walter Lozano
 wal...@vanguardiasur.com.ar wrote:
 On Mon, Mar 23, 2015 at 12:03 AM, Dinh Nguyen
 dingu...@opensource.altera.com wrote:
 On 3/19/15 4:27 PM, Walter Lozano wrote:
 On Mon, Mar 16, 2015 at 10:10 AM, Walter Lozano
 wal...@vanguardiasur.com.ar wrote:
 On Mon, Jan 5, 2015 at 6:21 AM, Steffen Trumtrar
 s.trumt...@pengutronix.de wrote:
 On Wed, Dec 24, 2014 at 08:11:52PM -0300, Walter Lozano wrote:
 This patch adds the DTS bindings for the adxl34x digital
 accelerometer.

 Signed-off-by: Walter Lozano wal...@vanguardiasur.com.ar
 ---
  arch/arm/boot/dts/socfpga_cyclone5_sockit.dts |   16 
  1 file changed, 16 insertions(+)

 diff --git a/arch/arm/boot/dts/socfpga_cyclone5_sockit.dts 
 b/arch/arm/boot/dts/socfpga_cyclone5_sockit.dts
 index 16ea6f5..d343e03 100644
 --- a/arch/arm/boot/dts/socfpga_cyclone5_sockit.dts
 +++ b/arch/arm/boot/dts/socfpga_cyclone5_sockit.dts
 @@ -60,6 +60,22 @@
   rxc-skew-ps = 2000;
  };

 +gpio2 {
 + status = okay;
 +};
 +
 +i2c1 {
 + status = okay;
 +
 + accel1: accelerometer@53 {
 + compatible = adxl34x;

 Shouldn't this be adi,adxl34x? At least that's what the documentation
 says.

 + reg = 0x53;
 +
 + interrupt-parent = portc;
 + interrupts = 3 2;
 + };
 +};
 +
  mmc0 {
   vmmc-supply = regulator_3_3v;
   vqmmc-supply = regulator_3_3v;

 I just gave it a short spin. I get some interrupts and the position 
 property
 changes, so it seems to work:

 Acked-by: Steffen Trumtrar s.trumt...@pengutronix.de


 Ping?
 How should this continue?

 Any news about this patch? It has been acked by Pavel Machek and
 Steffen Trumtrar a while ago.


 Sorry, since I wasn't CC on the original patch and somehow my filter
 didn't catch it, I didn't see this patch until now.

 No problem, I know it was my fault as I trust the output of 
 get_maintainer.pl


 Ping?

 --
 Walter Lozano, VanguardiaSur
 www.vanguardiasur.com.ar
 --
 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

 Now these commits are in:
   - 3a38958d2477b718 (Input: adxl34x - add OF match support) [1]
   - e465bf6fc55d5ce2 (DT: i2c: Deprecate adi,adxl34x compatible string) [2]

 you can use adi,adxl345 (or adi,adxl346, adi,adxl345).

 [1] 
 http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=3a38958d2477b718d1011fb88c24c4f264ee9700
 [2] 
 http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=e465bf6fc55d5ce21fb45a75c3fa613505e6be20

 Gr{oetje,eeting}s,

 Geert

 --
 Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- 
 ge...@linux-m68k.org

 In personal conversations with technical people, I call myself a hacker. But
 when I'm talking to journalists I just say programmer or something like 
 that.
 -- Linus Torvalds
 
 
 Thanks for the information Geert.
 
 Dihn, as I can see you have apply an old version of the this patch
 (V1), instead of V2 which has some format corrections suggested by
 Steffen Trumtrar. I addition, as Geert points, the compatibility
 string should now be updated.
 
 How would you like to proceed?
 

Your commit is staged for v4.2 in the arm-soc tree[1]. Please submit a
patch to correct it for an -rc.

[1]
https://git.kernel.org/cgit/linux/kernel/git/arm/arm-soc.git/commit/arch/arm/boot/dts/socfpga_cyclone5_sockit.dts?h=for-nextid=bf513bf04660321eb6181082def79ee1fd93b48f

Dinh
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 5/6] clk: sunxi: make use of of_clk_parent_fill helper function

[Dinh Nguyen]

On 06/11/2015 04:06 AM, Maxime Ripard wrote:
> Hi Dinh,
> 
> On Wed, Jun 10, 2015 at 04:49:24PM -0500, dingu...@opensource.altera.com 
> wrote:
>> From: Dinh Nguyen 
>>
>> Use of_clk_parent_fill to fill in the parent clock names' array.
>>
>> Signed-off-by: Dinh Nguyen 
>> Cc: Maxime Ripard 
>> Cc: "Emilio López" 
>> ---
>>  drivers/clk/sunxi/clk-a20-gmac.c|  3 +--
>>  drivers/clk/sunxi/clk-factors.c |  4 +---
>>  drivers/clk/sunxi/clk-sun6i-ar100.c |  3 +--
>>  drivers/clk/sunxi/clk-sunxi.c   | 10 ++
>>  4 files changed, 5 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/clk/sunxi/clk-a20-gmac.c 
>> b/drivers/clk/sunxi/clk-a20-gmac.c
>> index 0dcf4f2..a432edd 100644
>> --- a/drivers/clk/sunxi/clk-a20-gmac.c
>> +++ b/drivers/clk/sunxi/clk-a20-gmac.c
>> @@ -80,8 +80,7 @@ static void __init sun7i_a20_gmac_clk_setup(struct 
>> device_node *node)
>>  goto free_mux;
>>  
>>  /* gmac clock requires exactly 2 parents */
>> -parents[0] = of_clk_get_parent_name(node, 0);
>> -parents[1] = of_clk_get_parent_name(node, 1);
>> +of_clk_parent_fill(node, parents, 2);
>>  if (!parents[0] || !parents[1])
> 
> Maybe this check can be changed to something like:
> 
> if (of_clk_parent_fill(node, parents, 2) != 2)
>goto free_gate;
> 
> Would that make sense?
> 

Yes, that would work. Will edit for v2.

Thanks,
Dinh

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 5/6] clk: sunxi: make use of of_clk_parent_fill helper function

[Dinh Nguyen]

On 06/11/2015 04:06 AM, Maxime Ripard wrote:
 Hi Dinh,
 
 On Wed, Jun 10, 2015 at 04:49:24PM -0500, dingu...@opensource.altera.com 
 wrote:
 From: Dinh Nguyen dingu...@opensource.altera.com

 Use of_clk_parent_fill to fill in the parent clock names' array.

 Signed-off-by: Dinh Nguyen dingu...@opensource.altera.com
 Cc: Maxime Ripard maxime.rip...@free-electrons.com
 Cc: Emilio López emi...@elopez.com.ar
 ---
  drivers/clk/sunxi/clk-a20-gmac.c|  3 +--
  drivers/clk/sunxi/clk-factors.c |  4 +---
  drivers/clk/sunxi/clk-sun6i-ar100.c |  3 +--
  drivers/clk/sunxi/clk-sunxi.c   | 10 ++
  4 files changed, 5 insertions(+), 15 deletions(-)

 diff --git a/drivers/clk/sunxi/clk-a20-gmac.c 
 b/drivers/clk/sunxi/clk-a20-gmac.c
 index 0dcf4f2..a432edd 100644
 --- a/drivers/clk/sunxi/clk-a20-gmac.c
 +++ b/drivers/clk/sunxi/clk-a20-gmac.c
 @@ -80,8 +80,7 @@ static void __init sun7i_a20_gmac_clk_setup(struct 
 device_node *node)
  goto free_mux;
  
  /* gmac clock requires exactly 2 parents */
 -parents[0] = of_clk_get_parent_name(node, 0);
 -parents[1] = of_clk_get_parent_name(node, 1);
 +of_clk_parent_fill(node, parents, 2);
  if (!parents[0] || !parents[1])
 
 Maybe this check can be changed to something like:
 
 if (of_clk_parent_fill(node, parents, 2) != 2)
goto free_gate;
 
 Would that make sense?
 

Yes, that would work. Will edit for v2.

Thanks,
Dinh

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

Re: [PATCH v2] ARM: socfpga: add smp_ops.cpu_kill to make kexec/kdump available

[Dinh Nguyen]

On 06/10/2015 02:47 AM, Hiraku Toyooka wrote:
> Kexec_load syscall in ARM requires that machine-specific code
> has the smp_ops.cpu_kill() before loading kernel image.
> This patch adds the cpu_kill(), as a result, kexec reboot and
> kernel crash dump become available in mach-socfpga.
> 
> Signed-off-by: Hiraku Toyooka 
> Cc: Dinh Nguyen 
> Cc: Russell King 
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> ---
>  arch/arm/mach-socfpga/platsmp.c |   12 
>  1 file changed, 12 insertions(+)
> 

Applied!

Thanks,
Dinh

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2] ARM: socfpga: add smp_ops.cpu_kill to make kexec/kdump available

[Dinh Nguyen]

On 06/10/2015 02:47 AM, Hiraku Toyooka wrote:
 Kexec_load syscall in ARM requires that machine-specific code
 has the smp_ops.cpu_kill() before loading kernel image.
 This patch adds the cpu_kill(), as a result, kexec reboot and
 kernel crash dump become available in mach-socfpga.
 
 Signed-off-by: Hiraku Toyooka hiraku.toyooka...@hitachi.com
 Cc: Dinh Nguyen dingu...@opensource.altera.com
 Cc: Russell King li...@arm.linux.org.uk
 Cc: linux-arm-ker...@lists.infradead.org
 Cc: linux-kernel@vger.kernel.org
 ---
  arch/arm/mach-socfpga/platsmp.c |   12 
  1 file changed, 12 insertions(+)
 

Applied!

Thanks,
Dinh

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

Re: [RFC/PATCHv2 1/2] clk: of: helper for filling parent clock array and return num of parents

[Dinh Nguyen]

On 6/4/15 4:24 PM, Stephen Boyd wrote:
> On 06/04, dingu...@opensource.altera.com wrote:
>> From: Dinh Nguyen 
>>
>> Sprinkled all through the platform clock drivers are code like this to
>> fill the clock parent array:
>>
>> for (i = 0; i < num_parents; ++i)
>>  parent_names[i] = of_clk_get_parent_name(np, i);
>>
>> The of_clk_parent_fill() will do the same as the code above, and while
>> at it, return the number of parents as well since the logic of the
>> function is to the walk the clock node to look for the parent.
>>
>> Signed-off-by: Dinh Nguyen 
>> ---
>> v2: use unsigned int for size
>> ---
>>  drivers/clk/clk.c| 20 
>>  include/linux/clk-provider.h |  1 +
>>  2 files changed, 21 insertions(+)
>>
>> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
>> index 459ce9d..778bebd 100644
>> --- a/drivers/clk/clk.c
>> +++ b/drivers/clk/clk.c
>> @@ -3060,6 +3060,26 @@ const char *of_clk_get_parent_name(struct device_node 
>> *np, int index)
>>  }
>>  EXPORT_SYMBOL_GPL(of_clk_get_parent_name);
>>  
>> +/*
> 
> Need a double ** here for kernel doc.
> 

Ok...

>> + * of_clk_parent_fill(): Helper clock function that will fill the parent
>> + * clock's array and return the number of parents it found.
> 
> Please try to shorten this sentence.
> 
>   Fill @parents with names of @np's parents and return number of parents
> 
>> + * @np: Device node pointer associated with clock provider
>> + * @parents: pointer to char array that hold the parent's name
> 
> s/parent's name/parents' names/ ?
> 

Will address in v3.

>> + * @size: size of the parents array
>> + *
>> + * Returns number of parents for the clock node.
>> + */
>> +int of_clk_parent_fill(struct device_node *np, const char **parents, 
>> unsigned int size)
>> +{
>> +unsigned int i = 0;
>> +
>> +while (i < size && (parents[i] = of_clk_get_parent_name(np, i)) != NULL)
>> +i++;
>> +
>> +return i;
>> +}
>> +EXPORT_SYMBOL(of_clk_parent_fill);
> 
> Can this be GPL?
> 

Yes.

Thanks,
Dinh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCHv2 0/4] Add Altera Arria10 EDAC Support

[Dinh Nguyen]

On 6/5/15 6:02 AM, Borislav Petkov wrote:
> On Thu, Jun 04, 2015 at 09:28:44AM -0500, ttha...@opensource.altera.com wrote:
>> From: Thor Thayer 
>>
>> This series of patches adds support for the Arria10 EDAC. The
>> SDRAM controller and ECC registers are significantly different
>> from the CycloneV/ArriaV but common areas could be abstracted.
>>
>> Thor Thayer (4):
>>   edac, altera: Generalize driver to use DT Memory size
>>   edac, altera: Refactor EDAC for Altera CycloneV SoC.
>>   edac, altera: Addition of Arria10 EDAC
>>   arm: socfpga: dts: Arria10 SDRAM EDAC DTS additions.
>>
>>  .../bindings/arm/altera/socfpga-sdram-edac.txt |2 +-
>>  arch/arm/boot/dts/socfpga_arria10.dtsi |   11 +
>>  drivers/edac/altera_edac.c |  364 
>> 
>>  drivers/edac/altera_edac.h |  201 +++
>>  4 files changed, 437 insertions(+), 141 deletions(-)
>>  create mode 100644 drivers/edac/altera_edac.h
> 
> Ok, all applied, no need to redo them.
> 
> altera_edac still builds ok with the cross compiler here but you should
> doublecheck my final result just in case:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/bp/bp.git#for-next
> 

This builds fine for me.

Thanks,
Dinh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCHv2 2/4] edac, altera: Refactor EDAC for Altera CycloneV SoC.

[Dinh Nguyen]

Hi Boris,

On 6/5/15 4:17 AM, Borislav Petkov wrote:
> On Thu, Jun 04, 2015 at 05:27:28PM -0500, Dinh Nguyen wrote:
>> This is my mistake. I applied Alan Tull's patch for suspend-to-ram which
>> also touches drivers/edac/altera_edac.c.
>>
>> https://git.kernel.org/cgit/linux/kernel/git/dinguyen/linux.git/commit/?h=socfpga_for_next_arria10=d44a92eb044f4cc1abf34a790d647e9fa356d7b0
>>
>> I think I will have to ask Alan to split up his patch and send the part
>> that touches altera_edac.c to linux-edac.
> 
> Nah, that's fine. If it is that hunk adding altr_sdram_prepare(), he can
> simply add my ACK for the EDAC bits:
> 
> Acked-by: Borislav Petkov 
> 

Thanks alot for doing that! But I have sent you a separate patch for
this. It would have caused a merge conflict for v4.2 if I left the
suspend-to-ram patch as is.

Thanks,

Dinh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCHv6 0/2] socfpga: support suspend to ram*

[Dinh Nguyen]

On Thu, Jun 4, 2015 at 5:35 PM, Dinh Nguyen
 wrote:
> Hi Alan,
>
> On 06/02/2015 02:22 PM, Dinh Nguyen wrote:
>> On 06/02/2015 01:35 PM, Alan Tull wrote:
>>> Support suspend to ram on socfpga.
>>>   * allocate space in ocram using sram driver.
>>>   * Add a function in ocram to place DDR in self-refresh
>>> and suspend.
>>>   * Prevent suspend if EDAC is enabled.
>>>   * Add a device tree binding document for the Altera
>>> SOCFPGA SDRAM controller that is used to put DDR in
>>> self-refresh mode.
>>>
>>> Alan Tull (2):
>>>   ARM: socfpga: support suspend to ram
>>>   ARM: socfpga: dts: add sdram controller dt binding doc
>>>
>>>  .../arm/altera/socfpga-sdram-controller.txt|   12 ++
>>>  arch/arm/mach-socfpga/Kconfig  |   10 +-
>>>  arch/arm/mach-socfpga/Makefile |1 +
>>>  arch/arm/mach-socfpga/core.h   |6 +-
>>>  arch/arm/mach-socfpga/pm.c |  149 
>>> 
>>>  arch/arm/mach-socfpga/self-refresh.S   |  136 
>>> ++
>>>  arch/arm/mach-socfpga/socfpga.c|6 +-
>>>  drivers/edac/altera_edac.c |   20 +++
>>>  8 files changed, 337 insertions(+), 3 deletions(-)
>>>  create mode 100644 
>>> Documentation/devicetree/bindings/arm/altera/socfpga-sdram-controller.txt
>>>  create mode 100644 arch/arm/mach-socfpga/pm.c
>>>  create mode 100644 arch/arm/mach-socfpga/self-refresh.S
>>>
>>
>>
>> Applied.
>>
>
> I had to un-apply this patch because I noticed that it's touching
> drivers/edac/altera_edac.c. This part should be a separate patch and
> need to go through linux-edac.
>

Nevermind on this, I got an ack-by from Borislav Petkov for the edac
part. No need to split the patch up.

Dinh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCHv6 0/2] socfpga: support suspend to ram*

[Dinh Nguyen]

On Thu, Jun 4, 2015 at 5:35 PM, Dinh Nguyen
dingu...@opensource.altera.com wrote:
 Hi Alan,

 On 06/02/2015 02:22 PM, Dinh Nguyen wrote:
 On 06/02/2015 01:35 PM, Alan Tull wrote:
 Support suspend to ram on socfpga.
   * allocate space in ocram using sram driver.
   * Add a function in ocram to place DDR in self-refresh
 and suspend.
   * Prevent suspend if EDAC is enabled.
   * Add a device tree binding document for the Altera
 SOCFPGA SDRAM controller that is used to put DDR in
 self-refresh mode.

 Alan Tull (2):
   ARM: socfpga: support suspend to ram
   ARM: socfpga: dts: add sdram controller dt binding doc

  .../arm/altera/socfpga-sdram-controller.txt|   12 ++
  arch/arm/mach-socfpga/Kconfig  |   10 +-
  arch/arm/mach-socfpga/Makefile |1 +
  arch/arm/mach-socfpga/core.h   |6 +-
  arch/arm/mach-socfpga/pm.c |  149 
 
  arch/arm/mach-socfpga/self-refresh.S   |  136 
 ++
  arch/arm/mach-socfpga/socfpga.c|6 +-
  drivers/edac/altera_edac.c |   20 +++
  8 files changed, 337 insertions(+), 3 deletions(-)
  create mode 100644 
 Documentation/devicetree/bindings/arm/altera/socfpga-sdram-controller.txt
  create mode 100644 arch/arm/mach-socfpga/pm.c
  create mode 100644 arch/arm/mach-socfpga/self-refresh.S



 Applied.


 I had to un-apply this patch because I noticed that it's touching
 drivers/edac/altera_edac.c. This part should be a separate patch and
 need to go through linux-edac.


Nevermind on this, I got an ack-by from Borislav Petkov for the edac
part. No need to split the patch up.

Dinh
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCHv2 0/4] Add Altera Arria10 EDAC Support

[Dinh Nguyen]

On 6/5/15 6:02 AM, Borislav Petkov wrote:
 On Thu, Jun 04, 2015 at 09:28:44AM -0500, ttha...@opensource.altera.com wrote:
 From: Thor Thayer ttha...@opensource.altera.com

 This series of patches adds support for the Arria10 EDAC. The
 SDRAM controller and ECC registers are significantly different
 from the CycloneV/ArriaV but common areas could be abstracted.

 Thor Thayer (4):
   edac, altera: Generalize driver to use DT Memory size
   edac, altera: Refactor EDAC for Altera CycloneV SoC.
   edac, altera: Addition of Arria10 EDAC
   arm: socfpga: dts: Arria10 SDRAM EDAC DTS additions.

  .../bindings/arm/altera/socfpga-sdram-edac.txt |2 +-
  arch/arm/boot/dts/socfpga_arria10.dtsi |   11 +
  drivers/edac/altera_edac.c |  364 
 
  drivers/edac/altera_edac.h |  201 +++
  4 files changed, 437 insertions(+), 141 deletions(-)
  create mode 100644 drivers/edac/altera_edac.h
 
 Ok, all applied, no need to redo them.
 
 altera_edac still builds ok with the cross compiler here but you should
 doublecheck my final result just in case:
 
 git://git.kernel.org/pub/scm/linux/kernel/git/bp/bp.git#for-next
 

This builds fine for me.

Thanks,
Dinh
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCHv2 2/4] edac, altera: Refactor EDAC for Altera CycloneV SoC.

[Dinh Nguyen]

Hi Boris,

On 6/5/15 4:17 AM, Borislav Petkov wrote:
 On Thu, Jun 04, 2015 at 05:27:28PM -0500, Dinh Nguyen wrote:
 This is my mistake. I applied Alan Tull's patch for suspend-to-ram which
 also touches drivers/edac/altera_edac.c.

 https://git.kernel.org/cgit/linux/kernel/git/dinguyen/linux.git/commit/?h=socfpga_for_next_arria10id=d44a92eb044f4cc1abf34a790d647e9fa356d7b0

 I think I will have to ask Alan to split up his patch and send the part
 that touches altera_edac.c to linux-edac.
 
 Nah, that's fine. If it is that hunk adding altr_sdram_prepare(), he can
 simply add my ACK for the EDAC bits:
 
 Acked-by: Borislav Petkov b...@suse.de
 

Thanks alot for doing that! But I have sent you a separate patch for
this. It would have caused a merge conflict for v4.2 if I left the
suspend-to-ram patch as is.

Thanks,

Dinh
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC/PATCHv2 1/2] clk: of: helper for filling parent clock array and return num of parents

[Dinh Nguyen]

On 6/4/15 4:24 PM, Stephen Boyd wrote:
 On 06/04, dingu...@opensource.altera.com wrote:
 From: Dinh Nguyen dingu...@opensource.altera.com

 Sprinkled all through the platform clock drivers are code like this to
 fill the clock parent array:

 for (i = 0; i  num_parents; ++i)
  parent_names[i] = of_clk_get_parent_name(np, i);

 The of_clk_parent_fill() will do the same as the code above, and while
 at it, return the number of parents as well since the logic of the
 function is to the walk the clock node to look for the parent.

 Signed-off-by: Dinh Nguyen dingu...@opensource.altera.com
 ---
 v2: use unsigned int for size
 ---
  drivers/clk/clk.c| 20 
  include/linux/clk-provider.h |  1 +
  2 files changed, 21 insertions(+)

 diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
 index 459ce9d..778bebd 100644
 --- a/drivers/clk/clk.c
 +++ b/drivers/clk/clk.c
 @@ -3060,6 +3060,26 @@ const char *of_clk_get_parent_name(struct device_node 
 *np, int index)
  }
  EXPORT_SYMBOL_GPL(of_clk_get_parent_name);
  
 +/*
 
 Need a double ** here for kernel doc.
 

Ok...

 + * of_clk_parent_fill(): Helper clock function that will fill the parent
 + * clock's array and return the number of parents it found.
 
 Please try to shorten this sentence.
 
   Fill @parents with names of @np's parents and return number of parents
 
 + * @np: Device node pointer associated with clock provider
 + * @parents: pointer to char array that hold the parent's name
 
 s/parent's name/parents' names/ ?
 

Will address in v3.

 + * @size: size of the parents array
 + *
 + * Returns number of parents for the clock node.
 + */
 +int of_clk_parent_fill(struct device_node *np, const char **parents, 
 unsigned int size)
 +{
 +unsigned int i = 0;
 +
 +while (i  size  (parents[i] = of_clk_get_parent_name(np, i)) != NULL)
 +i++;
 +
 +return i;
 +}
 +EXPORT_SYMBOL(of_clk_parent_fill);
 
 Can this be GPL?
 

Yes.

Thanks,
Dinh
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCHv6 0/2] socfpga: support suspend to ram*

[Dinh Nguyen]

Hi Alan,

On 06/02/2015 02:22 PM, Dinh Nguyen wrote:
> On 06/02/2015 01:35 PM, Alan Tull wrote:
>> Support suspend to ram on socfpga.
>>   * allocate space in ocram using sram driver.
>>   * Add a function in ocram to place DDR in self-refresh
>> and suspend.
>>   * Prevent suspend if EDAC is enabled.
>>   * Add a device tree binding document for the Altera
>> SOCFPGA SDRAM controller that is used to put DDR in
>> self-refresh mode.
>>
>> Alan Tull (2):
>>   ARM: socfpga: support suspend to ram
>>   ARM: socfpga: dts: add sdram controller dt binding doc
>>
>>  .../arm/altera/socfpga-sdram-controller.txt|   12 ++
>>  arch/arm/mach-socfpga/Kconfig  |   10 +-
>>  arch/arm/mach-socfpga/Makefile |1 +
>>  arch/arm/mach-socfpga/core.h   |6 +-
>>  arch/arm/mach-socfpga/pm.c |  149 
>> 
>>  arch/arm/mach-socfpga/self-refresh.S   |  136 ++
>>  arch/arm/mach-socfpga/socfpga.c|6 +-
>>  drivers/edac/altera_edac.c |   20 +++
>>  8 files changed, 337 insertions(+), 3 deletions(-)
>>  create mode 100644 
>> Documentation/devicetree/bindings/arm/altera/socfpga-sdram-controller.txt
>>  create mode 100644 arch/arm/mach-socfpga/pm.c
>>  create mode 100644 arch/arm/mach-socfpga/self-refresh.S
>>
> 
> 
> Applied.
> 

I had to un-apply this patch because I noticed that it's touching
drivers/edac/altera_edac.c. This part should be a separate patch and
need to go through linux-edac.

Dinh

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCHv2 2/4] edac, altera: Refactor EDAC for Altera CycloneV SoC.

[Dinh Nguyen]

On 06/04/2015 05:06 PM, Borislav Petkov wrote:
> On Thu, Jun 04, 2015 at 04:34:49PM -0500, Thor Thayer wrote:
>> OK. I'll refactor and resend. I was using Altera's internal for-next branch.
> 
> Use this one:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/bp/bp.git#for-next
> 

This is my mistake. I applied Alan Tull's patch for suspend-to-ram which
also touches drivers/edac/altera_edac.c.

https://git.kernel.org/cgit/linux/kernel/git/dinguyen/linux.git/commit/?h=socfpga_for_next_arria10=d44a92eb044f4cc1abf34a790d647e9fa356d7b0

I think I will have to ask Alan to split up his patch and send the part
that touches altera_edac.c to linux-edac.

Sorry about the confusion.

Dinh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCHv2 2/4] edac, altera: Refactor EDAC for Altera CycloneV SoC.

[Dinh Nguyen]

On 06/04/2015 09:28 AM, ttha...@opensource.altera.com wrote:
> From: Thor Thayer 
> 
> The Arria10 SOC uses a completely different SDRAM controller from the
> earlier CycloneV and ArriaV SoCs. This patch abstracts the SDRAM bits
> for the CycloneV/ArriaV SoCs in preparation for the Arria10 support.
> 
> Signed-off-by: Thor Thayer 
> ---
> v2: Make c5_data static.
> ---
>  drivers/edac/altera_edac.c |  194 
> 
>  drivers/edac/altera_edac.h |  116 ++
>  2 files changed, 206 insertions(+), 104 deletions(-)
>  create mode 100644 drivers/edac/altera_edac.h
> 
> diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c
> index a9e7c69..a8350f6 100644
> --- a/drivers/edac/altera_edac.c
> +++ b/drivers/edac/altera_edac.c

What branch are you basing this patch on? I get the following error when
applied to both v4.1-rc6, linux-next, and linux-edac/linux-next.

Applying: edac, altera: Refactor EDAC for Altera CycloneV SoC.
error: patch failed: drivers/edac/altera_edac.c:400
error: drivers/edac/altera_edac.c: patch does not apply
Patch failed at 0001 edac, altera: Refactor EDAC for Altera CycloneV SoC.

Dinh



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCHv6 0/2] socfpga: support suspend to ram*

[Dinh Nguyen]

Hi Alan,

On 06/02/2015 02:22 PM, Dinh Nguyen wrote:
 On 06/02/2015 01:35 PM, Alan Tull wrote:
 Support suspend to ram on socfpga.
   * allocate space in ocram using sram driver.
   * Add a function in ocram to place DDR in self-refresh
 and suspend.
   * Prevent suspend if EDAC is enabled.
   * Add a device tree binding document for the Altera
 SOCFPGA SDRAM controller that is used to put DDR in
 self-refresh mode.

 Alan Tull (2):
   ARM: socfpga: support suspend to ram
   ARM: socfpga: dts: add sdram controller dt binding doc

  .../arm/altera/socfpga-sdram-controller.txt|   12 ++
  arch/arm/mach-socfpga/Kconfig  |   10 +-
  arch/arm/mach-socfpga/Makefile |1 +
  arch/arm/mach-socfpga/core.h   |6 +-
  arch/arm/mach-socfpga/pm.c |  149 
 
  arch/arm/mach-socfpga/self-refresh.S   |  136 ++
  arch/arm/mach-socfpga/socfpga.c|6 +-
  drivers/edac/altera_edac.c |   20 +++
  8 files changed, 337 insertions(+), 3 deletions(-)
  create mode 100644 
 Documentation/devicetree/bindings/arm/altera/socfpga-sdram-controller.txt
  create mode 100644 arch/arm/mach-socfpga/pm.c
  create mode 100644 arch/arm/mach-socfpga/self-refresh.S

 
 
 Applied.
 

I had to un-apply this patch because I noticed that it's touching
drivers/edac/altera_edac.c. This part should be a separate patch and
need to go through linux-edac.

Dinh

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

Re: [PATCHv2 2/4] edac, altera: Refactor EDAC for Altera CycloneV SoC.

[Dinh Nguyen]

On 06/04/2015 05:06 PM, Borislav Petkov wrote:
 On Thu, Jun 04, 2015 at 04:34:49PM -0500, Thor Thayer wrote:
 OK. I'll refactor and resend. I was using Altera's internal for-next branch.
 
 Use this one:
 
 git://git.kernel.org/pub/scm/linux/kernel/git/bp/bp.git#for-next
 

This is my mistake. I applied Alan Tull's patch for suspend-to-ram which
also touches drivers/edac/altera_edac.c.

https://git.kernel.org/cgit/linux/kernel/git/dinguyen/linux.git/commit/?h=socfpga_for_next_arria10id=d44a92eb044f4cc1abf34a790d647e9fa356d7b0

I think I will have to ask Alan to split up his patch and send the part
that touches altera_edac.c to linux-edac.

Sorry about the confusion.

Dinh
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCHv2 2/4] edac, altera: Refactor EDAC for Altera CycloneV SoC.

[Dinh Nguyen]

On 06/04/2015 09:28 AM, ttha...@opensource.altera.com wrote:
 From: Thor Thayer ttha...@opensource.altera.com
 
 The Arria10 SOC uses a completely different SDRAM controller from the
 earlier CycloneV and ArriaV SoCs. This patch abstracts the SDRAM bits
 for the CycloneV/ArriaV SoCs in preparation for the Arria10 support.
 
 Signed-off-by: Thor Thayer ttha...@opensource.altera.com
 ---
 v2: Make c5_data static.
 ---
  drivers/edac/altera_edac.c |  194 
 
  drivers/edac/altera_edac.h |  116 ++
  2 files changed, 206 insertions(+), 104 deletions(-)
  create mode 100644 drivers/edac/altera_edac.h
 
 diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c
 index a9e7c69..a8350f6 100644
 --- a/drivers/edac/altera_edac.c
 +++ b/drivers/edac/altera_edac.c

What branch are you basing this patch on? I get the following error when
applied to both v4.1-rc6, linux-next, and linux-edac/linux-next.

Applying: edac, altera: Refactor EDAC for Altera CycloneV SoC.
error: patch failed: drivers/edac/altera_edac.c:400
error: drivers/edac/altera_edac.c: patch does not apply
Patch failed at 0001 edac, altera: Refactor EDAC for Altera CycloneV SoC.

Dinh



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

Re: [PATCHv6 0/2] socfpga: support suspend to ram*

[Dinh Nguyen]

On 06/02/2015 01:35 PM, Alan Tull wrote:
> Support suspend to ram on socfpga.
>   * allocate space in ocram using sram driver.
>   * Add a function in ocram to place DDR in self-refresh
> and suspend.
>   * Prevent suspend if EDAC is enabled.
>   * Add a device tree binding document for the Altera
> SOCFPGA SDRAM controller that is used to put DDR in
> self-refresh mode.
> 
> Alan Tull (2):
>   ARM: socfpga: support suspend to ram
>   ARM: socfpga: dts: add sdram controller dt binding doc
> 
>  .../arm/altera/socfpga-sdram-controller.txt|   12 ++
>  arch/arm/mach-socfpga/Kconfig  |   10 +-
>  arch/arm/mach-socfpga/Makefile |1 +
>  arch/arm/mach-socfpga/core.h   |6 +-
>  arch/arm/mach-socfpga/pm.c |  149 
> 
>  arch/arm/mach-socfpga/self-refresh.S   |  136 ++
>  arch/arm/mach-socfpga/socfpga.c|6 +-
>  drivers/edac/altera_edac.c |   20 +++
>  8 files changed, 337 insertions(+), 3 deletions(-)
>  create mode 100644 
> Documentation/devicetree/bindings/arm/altera/socfpga-sdram-controller.txt
>  create mode 100644 arch/arm/mach-socfpga/pm.c
>  create mode 100644 arch/arm/mach-socfpga/self-refresh.S
> 


Applied.

Thanks Alan!

Dinh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCHv6 0/2] socfpga: support suspend to ram*

[Dinh Nguyen]

On 06/02/2015 01:35 PM, Alan Tull wrote:
 Support suspend to ram on socfpga.
   * allocate space in ocram using sram driver.
   * Add a function in ocram to place DDR in self-refresh
 and suspend.
   * Prevent suspend if EDAC is enabled.
   * Add a device tree binding document for the Altera
 SOCFPGA SDRAM controller that is used to put DDR in
 self-refresh mode.
 
 Alan Tull (2):
   ARM: socfpga: support suspend to ram
   ARM: socfpga: dts: add sdram controller dt binding doc
 
  .../arm/altera/socfpga-sdram-controller.txt|   12 ++
  arch/arm/mach-socfpga/Kconfig  |   10 +-
  arch/arm/mach-socfpga/Makefile |1 +
  arch/arm/mach-socfpga/core.h   |6 +-
  arch/arm/mach-socfpga/pm.c |  149 
 
  arch/arm/mach-socfpga/self-refresh.S   |  136 ++
  arch/arm/mach-socfpga/socfpga.c|6 +-
  drivers/edac/altera_edac.c |   20 +++
  8 files changed, 337 insertions(+), 3 deletions(-)
  create mode 100644 
 Documentation/devicetree/bindings/arm/altera/socfpga-sdram-controller.txt
  create mode 100644 arch/arm/mach-socfpga/pm.c
  create mode 100644 arch/arm/mach-socfpga/self-refresh.S
 


Applied.

Thanks Alan!

Dinh
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCHv5 1/2] ARM: socfpga: support suspend to ram

[Dinh Nguyen]

On 5/28/15 4:19 PM, Alan Tull wrote:
> Add code that requests that the sdr controller go into
> self-refresh mode.  This code is run from ocram.
> 
> Suspend-to-RAM and EDAC support are mutually exclusive on
> SOCFPGA.  If the EDAC is enabled, it will prevent the
> platform from going into suspend.
> 
> Example of how to request to suspend to ram:
>  $ echo enabled > \
> /sys/devices/soc/ffc02000.serial0/tty/ttyS0/power/wakeup
> 
>  $ echo -n mem > /sys/power/state
> 
> Signed-off-by: Alan Tull 
> Cc: Pavel Machek 
> Cc: Arnd Bergmann 
> Cc: Dinh Nguyen 
> Cc: Steffen Trumtrar 
> ---
> v2: use Generic on-chip SRAM driver to allocate ocram
> rm fncpy_align since generic allocator handles alignment
> check __arm_ioremap_exec return code
> check for NULL pointers
> add a comment regarding sdram controller configuration
> v3: fix renamed #define
> propagate socfpga_setup_ocram_self_refresh error code
> v4: Kconfig: don't need to select GENERIC_ALLOCATER
> add CONFIG_SOCFPGA_SUSPEND
> make s2r and EDAC support mutually exclusive
> socfpga.c: add sdr_ctl_base_addr
> return error if ocram not available in device tree
> update copyright years
> v5: remove compile time dependency
> edac driver will prevent suspend
> don't configure scu standby mode; done in enable_scu().
> fix comments about required sdram controller configuration
> ---
>  arch/arm/mach-socfpga/Kconfig|   10 ++-
>  arch/arm/mach-socfpga/Makefile   |1 +
>  arch/arm/mach-socfpga/core.h |6 +-
>  arch/arm/mach-socfpga/pm.c   |  149 
> ++
>  arch/arm/mach-socfpga/self-refresh.S |  136 +++
>  arch/arm/mach-socfpga/socfpga.c  |6 +-
>  drivers/edac/altera_edac.c   |   20 +
>  7 files changed, 325 insertions(+), 3 deletions(-)
>  create mode 100644 arch/arm/mach-socfpga/pm.c
>  create mode 100644 arch/arm/mach-socfpga/self-refresh.S
> 

Which branch are you basing this patch on? I got a trivial patch
conflict in Kconfig when I applied to arm-soc/for-next and v4.1-rc5.

Also, I'm getting these sparse warnings:

arch/arm/mach-socfpga/pm.c:86:25: warning: incorrect type in argument 1
(different address spaces)
arch/arm/mach-socfpga/pm.c:86:25:expected void *
arch/arm/mach-socfpga/pm.c:86:25:got void [noderef]
*[assigned] suspend_ocram_base
arch/arm/mach-socfpga/pm.c:108:52: warning: cast removes address space
of expression

Dinh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v4 1/2] ARM: socfpga: support suspend to ram

[Dinh Nguyen]

On 05/27/2015 03:25 PM, atull wrote:
> On Tue, 26 May 2015, Dinh Nguyen wrote:
> 
>> Hi Alan,
>>
>> On 5/22/15 1:02 PM, Alan Tull wrote:
>>> Add code that requests that the sdr controller go into
>>> self-refresh mode.  This code is run from ocram.
>>>
>>> This patch assumes that u-boot has already configured sdr:
>>>   sdr.ctrlcfg.lowpwreq.selfrfshmask = 3
>>>   sdr.ctrlcfg.lowpwrtiming.clkdisablecycles = 8
>>>   sdr.ctrlcfg.dramtiming4.selfrfshexit = 512
>>>
>>> Suspend-to-RAM and EDAC support are mutually exclusive on SOCFPGA
>>> platforms.  CONFIG_SOCFPGA_SUSPEND enables suspend-to-RAM and
>>> prevents selecting CONFIG_EDAC_ALTERA_MC.
>>>
>>> How to suspend to ram:
>>>  $ echo enabled > \
>>> /sys/devices/soc/ffc02000.serial0/tty/ttyS0/power/wakeup
>>>
>>>  $ echo -n mem > /sys/power/state
>>>
>>> Signed-off-by: Alan Tull 
>>> Cc: Pavel Machek 
>>> Cc: Arnd Bergmann 
>>> Cc: Dinh Nguyen 
>>> Cc: Steffen Trumtrar 
>>> ---
>>> v2: use Generic on-chip SRAM driver to allocate ocram
>>> rm fncpy_align since generic allocator handles alignment
>>> check __arm_ioremap_exec return code
>>> check for NULL pointers
>>> add a comment regarding sdram controller configuration
>>> v3: fix renamed #define
>>> propagate socfpga_setup_ocram_self_refresh error code
>>> v4: Kconfig: don't need to select GENERIC_ALLOCATER
>>> add CONFIG_SOCFPGA_SUSPEND
>>> make s2r and EDAC support mutually exclusive
>>> socfpga.c: add sdr_ctl_base_addr
>>> return error if ocram not available in device tree
>>> update copyright years
>>> ---
>>
>> 
>>
>>> +
>>> +static int socfpga_pm_suspend(unsigned long arg)
>>> +{
>>> +   u32 ret;
>>> +
>>> +   if (!sdr_ctl_base_addr || !socfpga_scu_base_addr)
>>> +   return -EFAULT;
>>> +
>>> +   ret = socfpga_sdram_self_refresh_in_ocram(
>>> +   (u32)sdr_ctl_base_addr, (u32)socfpga_scu_base_addr);
>>> +
>>
>> I had a patch that removed socfpga_scu_base_addr from being a global and
>> just a local variable in:
>>
>> f6e14376fb20 ARM: socfpga: use of_iomap to map the SCU
>>
>> This patch will be in v4.2 and is currently in arm-soc/next or
>> at my fork: kernel/git/dinguyen/linux.git socfpga_for_next_arria10
>>
>> So you will either need to make socfpga_scu_base_addr global again, or
>> you can use the asm instruction to get the SCU base addr.
>>
>> Sorry about that..
>>
>> Dinh
>>
> 
> I can't make socfpga_scu_base_addr a global from platsmp.c since that file
> may or may not be compiled in.

Ah, okay..
> 
> That leaves me with the option of adding the code that was removed from
> socfpga.c back where it was or taking that same code moving it to pm.c
> 

I think you only need the scu base address in
socfpga_sdram_self_refresh(), so you can probably just use the single
line assembly code to get it there.

Dinh

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCHv5 1/2] ARM: socfpga: support suspend to ram

[Dinh Nguyen]

On 5/28/15 4:19 PM, Alan Tull wrote:
 Add code that requests that the sdr controller go into
 self-refresh mode.  This code is run from ocram.
 
 Suspend-to-RAM and EDAC support are mutually exclusive on
 SOCFPGA.  If the EDAC is enabled, it will prevent the
 platform from going into suspend.
 
 Example of how to request to suspend to ram:
  $ echo enabled  \
 /sys/devices/soc/ffc02000.serial0/tty/ttyS0/power/wakeup
 
  $ echo -n mem  /sys/power/state
 
 Signed-off-by: Alan Tull at...@opensource.altera.com
 Cc: Pavel Machek pa...@denx.de
 Cc: Arnd Bergmann a...@arndb.de
 Cc: Dinh Nguyen dingu...@opensource.altera.com
 Cc: Steffen Trumtrar s.trumt...@pengutronix.de
 ---
 v2: use Generic on-chip SRAM driver to allocate ocram
 rm fncpy_align since generic allocator handles alignment
 check __arm_ioremap_exec return code
 check for NULL pointers
 add a comment regarding sdram controller configuration
 v3: fix renamed #define
 propagate socfpga_setup_ocram_self_refresh error code
 v4: Kconfig: don't need to select GENERIC_ALLOCATER
 add CONFIG_SOCFPGA_SUSPEND
 make s2r and EDAC support mutually exclusive
 socfpga.c: add sdr_ctl_base_addr
 return error if ocram not available in device tree
 update copyright years
 v5: remove compile time dependency
 edac driver will prevent suspend
 don't configure scu standby mode; done in enable_scu().
 fix comments about required sdram controller configuration
 ---
  arch/arm/mach-socfpga/Kconfig|   10 ++-
  arch/arm/mach-socfpga/Makefile   |1 +
  arch/arm/mach-socfpga/core.h |6 +-
  arch/arm/mach-socfpga/pm.c   |  149 
 ++
  arch/arm/mach-socfpga/self-refresh.S |  136 +++
  arch/arm/mach-socfpga/socfpga.c  |6 +-
  drivers/edac/altera_edac.c   |   20 +
  7 files changed, 325 insertions(+), 3 deletions(-)
  create mode 100644 arch/arm/mach-socfpga/pm.c
  create mode 100644 arch/arm/mach-socfpga/self-refresh.S
 

Which branch are you basing this patch on? I got a trivial patch
conflict in Kconfig when I applied to arm-soc/for-next and v4.1-rc5.

Also, I'm getting these sparse warnings:

arch/arm/mach-socfpga/pm.c:86:25: warning: incorrect type in argument 1
(different address spaces)
arch/arm/mach-socfpga/pm.c:86:25:expected void *noident
arch/arm/mach-socfpga/pm.c:86:25:got void [noderef]
asn:2*[assigned] suspend_ocram_base
arch/arm/mach-socfpga/pm.c:108:52: warning: cast removes address space
of expression

Dinh
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v4 1/2] ARM: socfpga: support suspend to ram

[Dinh Nguyen]

On 05/27/2015 03:25 PM, atull wrote:
 On Tue, 26 May 2015, Dinh Nguyen wrote:
 
 Hi Alan,

 On 5/22/15 1:02 PM, Alan Tull wrote:
 Add code that requests that the sdr controller go into
 self-refresh mode.  This code is run from ocram.

 This patch assumes that u-boot has already configured sdr:
   sdr.ctrlcfg.lowpwreq.selfrfshmask = 3
   sdr.ctrlcfg.lowpwrtiming.clkdisablecycles = 8
   sdr.ctrlcfg.dramtiming4.selfrfshexit = 512

 Suspend-to-RAM and EDAC support are mutually exclusive on SOCFPGA
 platforms.  CONFIG_SOCFPGA_SUSPEND enables suspend-to-RAM and
 prevents selecting CONFIG_EDAC_ALTERA_MC.

 How to suspend to ram:
  $ echo enabled  \
 /sys/devices/soc/ffc02000.serial0/tty/ttyS0/power/wakeup

  $ echo -n mem  /sys/power/state

 Signed-off-by: Alan Tull at...@opensource.altera.com
 Cc: Pavel Machek pa...@denx.de
 Cc: Arnd Bergmann a...@arndb.de
 Cc: Dinh Nguyen dingu...@opensource.altera.com
 Cc: Steffen Trumtrar s.trumt...@pengutronix.de
 ---
 v2: use Generic on-chip SRAM driver to allocate ocram
 rm fncpy_align since generic allocator handles alignment
 check __arm_ioremap_exec return code
 check for NULL pointers
 add a comment regarding sdram controller configuration
 v3: fix renamed #define
 propagate socfpga_setup_ocram_self_refresh error code
 v4: Kconfig: don't need to select GENERIC_ALLOCATER
 add CONFIG_SOCFPGA_SUSPEND
 make s2r and EDAC support mutually exclusive
 socfpga.c: add sdr_ctl_base_addr
 return error if ocram not available in device tree
 update copyright years
 ---

 snip

 +
 +static int socfpga_pm_suspend(unsigned long arg)
 +{
 +   u32 ret;
 +
 +   if (!sdr_ctl_base_addr || !socfpga_scu_base_addr)
 +   return -EFAULT;
 +
 +   ret = socfpga_sdram_self_refresh_in_ocram(
 +   (u32)sdr_ctl_base_addr, (u32)socfpga_scu_base_addr);
 +

 I had a patch that removed socfpga_scu_base_addr from being a global and
 just a local variable in:

 f6e14376fb20 ARM: socfpga: use of_iomap to map the SCU

 This patch will be in v4.2 and is currently in arm-soc/next or
 at my fork: kernel/git/dinguyen/linux.git socfpga_for_next_arria10

 So you will either need to make socfpga_scu_base_addr global again, or
 you can use the asm instruction to get the SCU base addr.

 Sorry about that..

 Dinh

 
 I can't make socfpga_scu_base_addr a global from platsmp.c since that file
 may or may not be compiled in.

Ah, okay..
 
 That leaves me with the option of adding the code that was removed from
 socfpga.c back where it was or taking that same code moving it to pm.c
 

I think you only need the scu base address in
socfpga_sdram_self_refresh(), so you can probably just use the single
line assembly code to get it there.

Dinh

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

Re: [PATCH v4 1/2] ARM: socfpga: support suspend to ram

[Dinh Nguyen]

Hi Alan,

On 5/22/15 1:02 PM, Alan Tull wrote:
> Add code that requests that the sdr controller go into
> self-refresh mode.  This code is run from ocram.
> 
> This patch assumes that u-boot has already configured sdr:
>   sdr.ctrlcfg.lowpwreq.selfrfshmask = 3
>   sdr.ctrlcfg.lowpwrtiming.clkdisablecycles = 8
>   sdr.ctrlcfg.dramtiming4.selfrfshexit = 512
> 
> Suspend-to-RAM and EDAC support are mutually exclusive on SOCFPGA
> platforms.  CONFIG_SOCFPGA_SUSPEND enables suspend-to-RAM and
> prevents selecting CONFIG_EDAC_ALTERA_MC.
> 
> How to suspend to ram:
>  $ echo enabled > \
> /sys/devices/soc/ffc02000.serial0/tty/ttyS0/power/wakeup
> 
>  $ echo -n mem > /sys/power/state
> 
> Signed-off-by: Alan Tull 
> Cc: Pavel Machek 
> Cc: Arnd Bergmann 
> Cc: Dinh Nguyen 
> Cc: Steffen Trumtrar 
> ---
> v2: use Generic on-chip SRAM driver to allocate ocram
> rm fncpy_align since generic allocator handles alignment
> check __arm_ioremap_exec return code
> check for NULL pointers
> add a comment regarding sdram controller configuration
> v3: fix renamed #define
> propagate socfpga_setup_ocram_self_refresh error code
> v4: Kconfig: don't need to select GENERIC_ALLOCATER
> add CONFIG_SOCFPGA_SUSPEND
> make s2r and EDAC support mutually exclusive
> socfpga.c: add sdr_ctl_base_addr
> return error if ocram not available in device tree
> update copyright years
> ---



> +
> +static int socfpga_pm_suspend(unsigned long arg)
> +{
> + u32 ret;
> +
> + if (!sdr_ctl_base_addr || !socfpga_scu_base_addr)
> + return -EFAULT;
> +
> + ret = socfpga_sdram_self_refresh_in_ocram(
> + (u32)sdr_ctl_base_addr, (u32)socfpga_scu_base_addr);
> +

I had a patch that removed socfpga_scu_base_addr from being a global and
just a local variable in:

f6e14376fb20 ARM: socfpga: use of_iomap to map the SCU

This patch will be in v4.2 and is currently in arm-soc/next or
at my fork: kernel/git/dinguyen/linux.git socfpga_for_next_arria10

So you will either need to make socfpga_scu_base_addr global again, or
you can use the asm instruction to get the SCU base addr.

Sorry about that..

Dinh

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v4 1/2] ARM: socfpga: support suspend to ram

[Dinh Nguyen]

Hi Alan,

On 5/22/15 1:02 PM, Alan Tull wrote:
 Add code that requests that the sdr controller go into
 self-refresh mode.  This code is run from ocram.
 
 This patch assumes that u-boot has already configured sdr:
   sdr.ctrlcfg.lowpwreq.selfrfshmask = 3
   sdr.ctrlcfg.lowpwrtiming.clkdisablecycles = 8
   sdr.ctrlcfg.dramtiming4.selfrfshexit = 512
 
 Suspend-to-RAM and EDAC support are mutually exclusive on SOCFPGA
 platforms.  CONFIG_SOCFPGA_SUSPEND enables suspend-to-RAM and
 prevents selecting CONFIG_EDAC_ALTERA_MC.
 
 How to suspend to ram:
  $ echo enabled  \
 /sys/devices/soc/ffc02000.serial0/tty/ttyS0/power/wakeup
 
  $ echo -n mem  /sys/power/state
 
 Signed-off-by: Alan Tull at...@opensource.altera.com
 Cc: Pavel Machek pa...@denx.de
 Cc: Arnd Bergmann a...@arndb.de
 Cc: Dinh Nguyen dingu...@opensource.altera.com
 Cc: Steffen Trumtrar s.trumt...@pengutronix.de
 ---
 v2: use Generic on-chip SRAM driver to allocate ocram
 rm fncpy_align since generic allocator handles alignment
 check __arm_ioremap_exec return code
 check for NULL pointers
 add a comment regarding sdram controller configuration
 v3: fix renamed #define
 propagate socfpga_setup_ocram_self_refresh error code
 v4: Kconfig: don't need to select GENERIC_ALLOCATER
 add CONFIG_SOCFPGA_SUSPEND
 make s2r and EDAC support mutually exclusive
 socfpga.c: add sdr_ctl_base_addr
 return error if ocram not available in device tree
 update copyright years
 ---

snip

 +
 +static int socfpga_pm_suspend(unsigned long arg)
 +{
 + u32 ret;
 +
 + if (!sdr_ctl_base_addr || !socfpga_scu_base_addr)
 + return -EFAULT;
 +
 + ret = socfpga_sdram_self_refresh_in_ocram(
 + (u32)sdr_ctl_base_addr, (u32)socfpga_scu_base_addr);
 +

I had a patch that removed socfpga_scu_base_addr from being a global and
just a local variable in:

f6e14376fb20 ARM: socfpga: use of_iomap to map the SCU

This patch will be in v4.2 and is currently in arm-soc/next or
at my fork: kernel/git/dinguyen/linux.git socfpga_for_next_arria10

So you will either need to make socfpga_scu_base_addr global again, or
you can use the asm instruction to get the SCU base addr.

Sorry about that..

Dinh

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

Re: [PATCH] dmaengine: pl300: enable the clock to PL330 dma

[Dinh Nguyen]

On 05/20/2015 07:35 PM, Krzysztof Kozlowski wrote:
> On 21.05.2015 09:16, Krzysztof Kozlowski wrote:
>> On 21.05.2015 05:30, Dinh Nguyen wrote:
>>> Hi Krzysztof,
>>>
>>> On 05/05/2015 02:22 PM, Dinh Nguyen wrote:
>>>> On 05/05/2015 09:56 AM, Dinh Nguyen wrote:
>>>>> On 05/04/2015 10:55 PM, Krzysztof Kozlowski wrote:
>>>>>> 2015-05-05 4:52 GMT+09:00 Dinh Nguyen :
>>>>>>> On 05/04/2015 09:06 AM, Dinh Nguyen wrote:
>>>>>>>> +CC Olof
>>>>>>>>
>>>>>>>> On 5/4/15 8:50 AM, Krzysztof Kozlowski wrote:
>>>>>>>>> 2015-05-04 22:28 GMT+09:00 Dinh Nguyen 
>>>>>>>>> :
>>>>>>>>>> Hi Krzystof,
>>>>>>>>>>
>>>>>>>>>> On 5/4/15 12:30 AM, Krzysztof Kozlowski wrote:
>>>>>>>>>>> 2015-05-04 13:28 GMT+09:00  :
>>>>>>>>>>>> From: Dinh Nguyen 
>>>>>>>>>>>>
>>>>>>>>>>>> Turn on the clock to the PL330 DMA if there is a clock node 
>>>>>>>>>>>> provided.
>>>>>>>>>>>
>>>>>>>>>>> Why? There is no explanation in the patch for this important 
>>>>>>>>>>> question - why?
>>>>>>>>>>>
>>>>>>>>>>> Amba bus already does this and provide a wrapper function.
>>>>>>>>>>> Additionally that would mess up with runtime PM and clock
>>>>>>>>>>> enable/disable.
>>>>>>>>>>
>>>>>>>>>> I don't see the clock for the DMA getting turned on at all, which is 
>>>>>>>>>> why
>>>>>>>>>> after the kernel has booted, the filesystem tries to open up a serial
>>>>>>>>>> port using DMA and the system hangs. The failure is seen here:
>>>>>>>>>>
>>>>>>>>>> http://arm-soc.lixom.net/bootlogs/next/next-20150504/socfpga-arm-multi_v7_defconfig.html
>>>>>>>>>
>>>>>>>>> Thanks!
>>>>>>>>>
>>>>>>>>> The amba bus and pl330 should enable the clock and then disable it
>>>>>>>>> after probing:
>>>>>>>>> static int amba_probe(struct device *dev)
>>>>>>>>> {
>>>>>>>>> ...
>>>>>>>>> ret = amba_get_enable_pclk(pcdev);
>>>>>>>>> ...
>>>>>>>>>
>>>>>>>>> I wonder why do you think it is not enabled at all?
>>>>>>>>
>>>>>>>> I've checked it down to the register level that the gate for this clock
>>>>>>>> does not get set.
>>>>>>>>
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> This only happens with the multi_v7_defconfig, because the PL330 DMA 
>>>>>>>>>> is
>>>>>>>>>> getting built into the kernel, while the socfpga_defconfig does not
>>>>>>>>>> enable the PL330.
>>>>>>>>>
>>>>>>>>> It makes sense. If pl330 driver is not enabled then necessary clocks
>>>>>>>>> are turned on by bootloader. Probing pl330 effectively disables the
>>>>>>>>> clock (if DMA is not used).
>>>>>>>>>
>>>>>>>>>> The DTS for the socfpga platform looks like this:
>>>>>>>>>>
>>>>>>>>>> pdma: pdma@ffe01000 {
>>>>>>>>>> compatible = "arm,pl330", "arm,primecell";
>>>>>>>>>> reg = <0xffe01000 0x1000>;
>>>>>>>>>> interrupts = <0 104 4>,
>>>>>>>>>> <0 105 4>,
>>>>>>>>>> ...
>>>>>>>>>> #dma-cells = <1>;
>>>>>>>>>> #dma-channels = <8>;
>>>>>>>>>> #dma-requests = <32>;
>>>>>>>>>>

Re: [PATCH] dmaengine: pl330: Fix hang on dmaengine_terminate_all on certain boards

[Dinh Nguyen]

Hi Krzysztof,

On 05/20/2015 07:34 PM, Krzysztof Kozlowski wrote:
> The pl330 device could hang infinitely on certain boards when DMA
> channels are terminated.
> 
> It was caused by lack of runtime resume when executing
> pl330_terminate_all() which calls the _stop() function. _stop() accesses
> device register and can loop infinitely while checking for device state.
> 
> The hang was confirmed by Dinh Nguyen on Altera SOCFPGA Cyclone V
> board during boot. It can be also triggered with:
> 
> $ echo 1 > /sys/module/dmatest/parameters/iterations
> $ echo dma1chan0 > /sys/module/dmatest/parameters/channel
> $ echo 1 > /sys/module/dmatest/parameters/run
> $ sleep 1
> $ cat /sys/module/dmatest/parameters/run
> 
> Reported-by: Dinh Nguyen 
> Signed-off-by: Krzysztof Kozlowski 
> Fixes: ae43b3289186 ("ARM: 8202/1: dmaengine: pl330: Add runtime Power 
> Management support v12")
> Cc: 
> Cc: Dinh Nguyen 
> ---
>  drivers/dma/pl330.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
> index a7d9d3029b14..340f9e607cd8 100644
> --- a/drivers/dma/pl330.c
> +++ b/drivers/dma/pl330.c
> @@ -2127,6 +2127,7 @@ static int pl330_terminate_all(struct dma_chan *chan)
>   struct pl330_dmac *pl330 = pch->dmac;
>   LIST_HEAD(list);
>  
> + pm_runtime_get_sync(pl330->ddma.dev);
>   spin_lock_irqsave(>lock, flags);
>   spin_lock(>lock);
>   _stop(pch->thread);
> @@ -2151,6 +2152,8 @@ static int pl330_terminate_all(struct dma_chan *chan)
>   list_splice_tail_init(>work_list, >desc_pool);
>   list_splice_tail_init(>completed_list, >desc_pool);
>   spin_unlock_irqrestore(>lock, flags);
> + pm_runtime_mark_last_busy(pl330->ddma.dev);
> + pm_runtime_put_autosuspend(pl330->ddma.dev);
>  
>   return 0;
>  }
> 

This patch indeeds fixes the pl330 DMA issue for the SoCFPGA platform.
The multi_v7_defconfig now completely boots on the platform with this patch.

Feel free to add:

Tested-by: Dinh Nguyen 

Thanks,
Dinh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] dmaengine: pl330: Fix hang on dmaengine_terminate_all on certain boards

[Dinh Nguyen]

Hi Krzysztof,

On 05/20/2015 07:34 PM, Krzysztof Kozlowski wrote:
 The pl330 device could hang infinitely on certain boards when DMA
 channels are terminated.
 
 It was caused by lack of runtime resume when executing
 pl330_terminate_all() which calls the _stop() function. _stop() accesses
 device register and can loop infinitely while checking for device state.
 
 The hang was confirmed by Dinh Nguyen on Altera SOCFPGA Cyclone V
 board during boot. It can be also triggered with:
 
 $ echo 1  /sys/module/dmatest/parameters/iterations
 $ echo dma1chan0  /sys/module/dmatest/parameters/channel
 $ echo 1  /sys/module/dmatest/parameters/run
 $ sleep 1
 $ cat /sys/module/dmatest/parameters/run
 
 Reported-by: Dinh Nguyen dingu...@opensource.altera.com
 Signed-off-by: Krzysztof Kozlowski k.kozlow...@samsung.com
 Fixes: ae43b3289186 (ARM: 8202/1: dmaengine: pl330: Add runtime Power 
 Management support v12)
 Cc: sta...@vger.kernel.org
 Cc: Dinh Nguyen dingu...@opensource.altera.com
 ---
  drivers/dma/pl330.c | 3 +++
  1 file changed, 3 insertions(+)
 
 diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
 index a7d9d3029b14..340f9e607cd8 100644
 --- a/drivers/dma/pl330.c
 +++ b/drivers/dma/pl330.c
 @@ -2127,6 +2127,7 @@ static int pl330_terminate_all(struct dma_chan *chan)
   struct pl330_dmac *pl330 = pch-dmac;
   LIST_HEAD(list);
  
 + pm_runtime_get_sync(pl330-ddma.dev);
   spin_lock_irqsave(pch-lock, flags);
   spin_lock(pl330-lock);
   _stop(pch-thread);
 @@ -2151,6 +2152,8 @@ static int pl330_terminate_all(struct dma_chan *chan)
   list_splice_tail_init(pch-work_list, pl330-desc_pool);
   list_splice_tail_init(pch-completed_list, pl330-desc_pool);
   spin_unlock_irqrestore(pch-lock, flags);
 + pm_runtime_mark_last_busy(pl330-ddma.dev);
 + pm_runtime_put_autosuspend(pl330-ddma.dev);
  
   return 0;
  }
 

This patch indeeds fixes the pl330 DMA issue for the SoCFPGA platform.
The multi_v7_defconfig now completely boots on the platform with this patch.

Feel free to add:

Tested-by: Dinh Nguyen dingu...@opensource.altera.com

Thanks,
Dinh
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] dmaengine: pl300: enable the clock to PL330 dma

[Dinh Nguyen]

On 05/20/2015 07:35 PM, Krzysztof Kozlowski wrote:
 On 21.05.2015 09:16, Krzysztof Kozlowski wrote:
 On 21.05.2015 05:30, Dinh Nguyen wrote:
 Hi Krzysztof,

 On 05/05/2015 02:22 PM, Dinh Nguyen wrote:
 On 05/05/2015 09:56 AM, Dinh Nguyen wrote:
 On 05/04/2015 10:55 PM, Krzysztof Kozlowski wrote:
 2015-05-05 4:52 GMT+09:00 Dinh Nguyen dingu...@opensource.altera.com:
 On 05/04/2015 09:06 AM, Dinh Nguyen wrote:
 +CC Olof

 On 5/4/15 8:50 AM, Krzysztof Kozlowski wrote:
 2015-05-04 22:28 GMT+09:00 Dinh Nguyen 
 dingu...@opensource.altera.com:
 Hi Krzystof,

 On 5/4/15 12:30 AM, Krzysztof Kozlowski wrote:
 2015-05-04 13:28 GMT+09:00  dingu...@opensource.altera.com:
 From: Dinh Nguyen dingu...@opensource.altera.com

 Turn on the clock to the PL330 DMA if there is a clock node 
 provided.

 Why? There is no explanation in the patch for this important 
 question - why?

 Amba bus already does this and provide a wrapper function.
 Additionally that would mess up with runtime PM and clock
 enable/disable.

 I don't see the clock for the DMA getting turned on at all, which is 
 why
 after the kernel has booted, the filesystem tries to open up a serial
 port using DMA and the system hangs. The failure is seen here:

 http://arm-soc.lixom.net/bootlogs/next/next-20150504/socfpga-arm-multi_v7_defconfig.html

 Thanks!

 The amba bus and pl330 should enable the clock and then disable it
 after probing:
 static int amba_probe(struct device *dev)
 {
 ...
 ret = amba_get_enable_pclk(pcdev);
 ...

 I wonder why do you think it is not enabled at all?

 I've checked it down to the register level that the gate for this clock
 does not get set.



 This only happens with the multi_v7_defconfig, because the PL330 DMA 
 is
 getting built into the kernel, while the socfpga_defconfig does not
 enable the PL330.

 It makes sense. If pl330 driver is not enabled then necessary clocks
 are turned on by bootloader. Probing pl330 effectively disables the
 clock (if DMA is not used).

 The DTS for the socfpga platform looks like this:

 pdma: pdma@ffe01000 {
 compatible = arm,pl330, arm,primecell;
 reg = 0xffe01000 0x1000;
 interrupts = 0 104 4,
 0 105 4,
 ...
 #dma-cells = 1;
 #dma-channels = 8;
 #dma-requests = 32;
 clocks = l4_main_clk;
 clock-names = apb_pclk;
 };

 Perhaps I have the wrong designation for clock-names and the amba 
 bus is
 not able to pick up the correct clock?

 I have two ideas:
 1. Is this really the clock for the DMA? If DMA is not used then
 disabling it should be OK.

 Yes, this is the clock for the DMA. Yeah, leaving this clock off is
 fine, until the DMA gets used. Up until v4.0, SoCFPGA was not using the
 DMA at all, but in v4.0, there was a patch to assign the UARTs to it's
 DMA channel.

 https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/arch/arm/boot/dts/socfpga.dtsi?id=78c03c7af89721bd8a4428408a8cc7b53972e4b8

 2. Disabling the clock may effectively disable its parent or
 grandparent if there are not more users. Maybe some other driver needs
 these parents to be enabled? This was the issue for at least one
 similar error (on Exynos boards).


 I'll check up on these issues. When I was debugging this issue, the
 l4_main_clk is only used by the DMA, so it was not getting turned on by
 an other drivers.


 Ah, it looks like perhaps there's a problem with the serial driver and
 suspend/resume? If disable CONFIG_PM, then the DMA seems to be working
 fine with the debug uart. It appears the DMA is getting suspended and
 doesn't get resumed.


 You mean runtime PM suspend and resume or system sleep? During boot
 only the first one should happen.

 It's runtime PM suspend/resume.


 Could you test the DMA with dmatest? Disable the DMA in UART and
 compile with CONFIG_DMATEST. Syntax for testing is here:
 Documentation/dmaengine/dmatest.txt


 # echo Y  /sys/module/dmatest/parameters/run
 [   93.143775] dmatest: Started 1 threads using dma0chan0
 [   93.149227] pm_generic_runtime_resume
 [   93.153334] dmatest: Started 1 threads using dma0chan1
 [   93.159380] dmatest: Started 1 threads using dma0chan2
 [   93.165041] dmatest: Started 1 threads using dma0chan3
 [   93.170280] dmatest: Started 1 threads using dma0chan4
 [   93.175996] dmatest: Started 1 threads using dma0chan5
 [   93.181642] dmatest: Started 1 threads using dma0chan6
 [   93.188754] dmatest: dma0chan1-copy0: summary 10 tests, 0 failures
 282 iops 2008 KB/s (0)
 [   93.197091] dmatest: Started 1 threads using dma0chan7
 [   93.199353] dmatest: dma0chan3-copy0: summary 10 tests, 0 failures
 297 iops 2260 KB/s (0)
 [   93.205407] dmatest: dma0chan0-copy0: summary 10 tests, 0 failures
 177 iops 1364 KB/s (0)
 [   93.215599] dmatest: dma0chan2-copy0: summary 10 tests, 0 failures
 196 iops 1450 KB/s (0)
 [   93.219994] dmatest: dma0chan4-copy0: summary 10 tests, 0 failures
 225 iops 1554

Re: [PATCH] dmaengine: pl300: enable the clock to PL330 dma

[Dinh Nguyen]

Hi Krzysztof,

On 05/05/2015 02:22 PM, Dinh Nguyen wrote:
> On 05/05/2015 09:56 AM, Dinh Nguyen wrote:
>> On 05/04/2015 10:55 PM, Krzysztof Kozlowski wrote:
>>> 2015-05-05 4:52 GMT+09:00 Dinh Nguyen :
>>>> On 05/04/2015 09:06 AM, Dinh Nguyen wrote:
>>>>> +CC Olof
>>>>>
>>>>> On 5/4/15 8:50 AM, Krzysztof Kozlowski wrote:
>>>>>> 2015-05-04 22:28 GMT+09:00 Dinh Nguyen :
>>>>>>> Hi Krzystof,
>>>>>>>
>>>>>>> On 5/4/15 12:30 AM, Krzysztof Kozlowski wrote:
>>>>>>>> 2015-05-04 13:28 GMT+09:00  :
>>>>>>>>> From: Dinh Nguyen 
>>>>>>>>>
>>>>>>>>> Turn on the clock to the PL330 DMA if there is a clock node provided.
>>>>>>>>
>>>>>>>> Why? There is no explanation in the patch for this important question 
>>>>>>>> - why?
>>>>>>>>
>>>>>>>> Amba bus already does this and provide a wrapper function.
>>>>>>>> Additionally that would mess up with runtime PM and clock
>>>>>>>> enable/disable.
>>>>>>>
>>>>>>> I don't see the clock for the DMA getting turned on at all, which is why
>>>>>>> after the kernel has booted, the filesystem tries to open up a serial
>>>>>>> port using DMA and the system hangs. The failure is seen here:
>>>>>>>
>>>>>>> http://arm-soc.lixom.net/bootlogs/next/next-20150504/socfpga-arm-multi_v7_defconfig.html
>>>>>>
>>>>>> Thanks!
>>>>>>
>>>>>> The amba bus and pl330 should enable the clock and then disable it
>>>>>> after probing:
>>>>>> static int amba_probe(struct device *dev)
>>>>>> {
>>>>>> ...
>>>>>> ret = amba_get_enable_pclk(pcdev);
>>>>>> ...
>>>>>>
>>>>>> I wonder why do you think it is not enabled at all?
>>>>>
>>>>> I've checked it down to the register level that the gate for this clock
>>>>> does not get set.
>>>>>
>>>>>>
>>>>>>>
>>>>>>> This only happens with the multi_v7_defconfig, because the PL330 DMA is
>>>>>>> getting built into the kernel, while the socfpga_defconfig does not
>>>>>>> enable the PL330.
>>>>>>
>>>>>> It makes sense. If pl330 driver is not enabled then necessary clocks
>>>>>> are turned on by bootloader. Probing pl330 effectively disables the
>>>>>> clock (if DMA is not used).
>>>>>>
>>>>>>> The DTS for the socfpga platform looks like this:
>>>>>>>
>>>>>>> pdma: pdma@ffe01000 {
>>>>>>> compatible = "arm,pl330", "arm,primecell";
>>>>>>> reg = <0xffe01000 0x1000>;
>>>>>>> interrupts = <0 104 4>,
>>>>>>> <0 105 4>,
>>>>>>> ...
>>>>>>> #dma-cells = <1>;
>>>>>>> #dma-channels = <8>;
>>>>>>> #dma-requests = <32>;
>>>>>>> clocks = <_main_clk>;
>>>>>>> clock-names = "apb_pclk";
>>>>>>> };
>>>>>>>
>>>>>>> Perhaps I have the wrong designation for clock-names and the amba bus is
>>>>>>> not able to pick up the correct clock?
>>>>>>
>>>>>> I have two ideas:
>>>>>> 1. Is this really the clock for the DMA? If DMA is not used then
>>>>>> disabling it should be OK.
>>>>>
>>>>> Yes, this is the clock for the DMA. Yeah, leaving this clock off is
>>>>> fine, until the DMA gets used. Up until v4.0, SoCFPGA was not using the
>>>>> DMA at all, but in v4.0, there was a patch to assign the UARTs to it's
>>>>> DMA channel.
>>>>>
>>>>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/arch/arm/boot/dts/socfpga.dtsi?id=78c03c7af89721bd8a4428408a8cc7b53972e4b8
>>>>>
>>>>>> 2. Disabling the clock may e

Re: [PATCH] dmaengine: pl300: enable the clock to PL330 dma

[Dinh Nguyen]

Hi Krzysztof,

On 05/05/2015 02:22 PM, Dinh Nguyen wrote:
 On 05/05/2015 09:56 AM, Dinh Nguyen wrote:
 On 05/04/2015 10:55 PM, Krzysztof Kozlowski wrote:
 2015-05-05 4:52 GMT+09:00 Dinh Nguyen dingu...@opensource.altera.com:
 On 05/04/2015 09:06 AM, Dinh Nguyen wrote:
 +CC Olof

 On 5/4/15 8:50 AM, Krzysztof Kozlowski wrote:
 2015-05-04 22:28 GMT+09:00 Dinh Nguyen dingu...@opensource.altera.com:
 Hi Krzystof,

 On 5/4/15 12:30 AM, Krzysztof Kozlowski wrote:
 2015-05-04 13:28 GMT+09:00  dingu...@opensource.altera.com:
 From: Dinh Nguyen dingu...@opensource.altera.com

 Turn on the clock to the PL330 DMA if there is a clock node provided.

 Why? There is no explanation in the patch for this important question 
 - why?

 Amba bus already does this and provide a wrapper function.
 Additionally that would mess up with runtime PM and clock
 enable/disable.

 I don't see the clock for the DMA getting turned on at all, which is why
 after the kernel has booted, the filesystem tries to open up a serial
 port using DMA and the system hangs. The failure is seen here:

 http://arm-soc.lixom.net/bootlogs/next/next-20150504/socfpga-arm-multi_v7_defconfig.html

 Thanks!

 The amba bus and pl330 should enable the clock and then disable it
 after probing:
 static int amba_probe(struct device *dev)
 {
 ...
 ret = amba_get_enable_pclk(pcdev);
 ...

 I wonder why do you think it is not enabled at all?

 I've checked it down to the register level that the gate for this clock
 does not get set.



 This only happens with the multi_v7_defconfig, because the PL330 DMA is
 getting built into the kernel, while the socfpga_defconfig does not
 enable the PL330.

 It makes sense. If pl330 driver is not enabled then necessary clocks
 are turned on by bootloader. Probing pl330 effectively disables the
 clock (if DMA is not used).

 The DTS for the socfpga platform looks like this:

 pdma: pdma@ffe01000 {
 compatible = arm,pl330, arm,primecell;
 reg = 0xffe01000 0x1000;
 interrupts = 0 104 4,
 0 105 4,
 ...
 #dma-cells = 1;
 #dma-channels = 8;
 #dma-requests = 32;
 clocks = l4_main_clk;
 clock-names = apb_pclk;
 };

 Perhaps I have the wrong designation for clock-names and the amba bus is
 not able to pick up the correct clock?

 I have two ideas:
 1. Is this really the clock for the DMA? If DMA is not used then
 disabling it should be OK.

 Yes, this is the clock for the DMA. Yeah, leaving this clock off is
 fine, until the DMA gets used. Up until v4.0, SoCFPGA was not using the
 DMA at all, but in v4.0, there was a patch to assign the UARTs to it's
 DMA channel.

 https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/arch/arm/boot/dts/socfpga.dtsi?id=78c03c7af89721bd8a4428408a8cc7b53972e4b8

 2. Disabling the clock may effectively disable its parent or
 grandparent if there are not more users. Maybe some other driver needs
 these parents to be enabled? This was the issue for at least one
 similar error (on Exynos boards).


 I'll check up on these issues. When I was debugging this issue, the
 l4_main_clk is only used by the DMA, so it was not getting turned on by
 an other drivers.


 Ah, it looks like perhaps there's a problem with the serial driver and
 suspend/resume? If disable CONFIG_PM, then the DMA seems to be working
 fine with the debug uart. It appears the DMA is getting suspended and
 doesn't get resumed.


 You mean runtime PM suspend and resume or system sleep? During boot
 only the first one should happen.

 It's runtime PM suspend/resume.


 Could you test the DMA with dmatest? Disable the DMA in UART and
 compile with CONFIG_DMATEST. Syntax for testing is here:
 Documentation/dmaengine/dmatest.txt


 # echo Y  /sys/module/dmatest/parameters/run
 [   93.143775] dmatest: Started 1 threads using dma0chan0
 [   93.149227] pm_generic_runtime_resume
 [   93.153334] dmatest: Started 1 threads using dma0chan1
 [   93.159380] dmatest: Started 1 threads using dma0chan2
 [   93.165041] dmatest: Started 1 threads using dma0chan3
 [   93.170280] dmatest: Started 1 threads using dma0chan4
 [   93.175996] dmatest: Started 1 threads using dma0chan5
 [   93.181642] dmatest: Started 1 threads using dma0chan6
 [   93.188754] dmatest: dma0chan1-copy0: summary 10 tests, 0 failures
 282 iops 2008 KB/s (0)
 [   93.197091] dmatest: Started 1 threads using dma0chan7
 [   93.199353] dmatest: dma0chan3-copy0: summary 10 tests, 0 failures
 297 iops 2260 KB/s (0)
 [   93.205407] dmatest: dma0chan0-copy0: summary 10 tests, 0 failures
 177 iops 1364 KB/s (0)
 [   93.215599] dmatest: dma0chan2-copy0: summary 10 tests, 0 failures
 196 iops 1450 KB/s (0)
 [   93.219994] dmatest: dma0chan4-copy0: summary 10 tests, 0 failures
 225 iops 1554 KB/s (0)
 [   93.224322] dmatest: dma0chan5-copy0: summary 10 tests, 0 failures
 231 iops 1948 KB/s (0)
 [   93.230065] dmatest: dma0chan6-copy0: summary

Re: [PATCHv3 2/4] clk: socfpga: add a clock driver for the Arria 10 platform

[Dinh Nguyen]

On 5/19/15 4:50 PM, Stephen Boyd wrote:
> On 05/19/15 09:29, Dinh Nguyen wrote:
>>
>> On 5/15/15 7:52 PM, Stephen Boyd wrote:
>>> On 05/07, dingu...@opensource.altera.com wrote:
>>>> +
>>>> +static int socfpga_clk_prepare(struct clk_hw *hwclk)
>>>> +{
>>>> +  struct socfpga_gate_clk *socfpgaclk = to_socfpga_gate_clk(hwclk);
>>>> +  struct regmap *sys_mgr_base_addr;
>>>> +  int i;
>>>> +  u32 hs_timing;
>>>> +  u32 clk_phase[2];
>>>> +
>>>> +  if (socfpgaclk->clk_phase[0] || socfpgaclk->clk_phase[1]) {
>>>> +  sys_mgr_base_addr = 
>>>> syscon_regmap_lookup_by_compatible("altr,sys-mgr");
>>>> +  if (IS_ERR(sys_mgr_base_addr)) {
>>> Is there a reason the syscon is grabbed lazily in prepare? Why
>>> not get it before registering this clock?
>> This syscon node is only associated with clocks that have a clk-phase
>> property, which on the SoCFPGA platform, is the SD/MMC clocks. The way
>> to implement this went through quite a few rounds of discussion for the
>> Cyclone5/Arria5 platform before settling to this method.
>>
>> The reason why syscon is grabbed here is that the setting of the clock
>> phase must be done before enabling of the clock, so it seem that prepare
>> was a good place. Should this be move moved to the socfpga_gate_init()
>> instead?
> 
> I was expecting the regmap to be found before the clock is registered
> and stored away into the  socfpga_gate_clk structure. Getting the regmap
> during prepare is akin to ioremapping a register region during prepare,
> which doesn't sound right at all. Maybe there's some good reason in the
> earlier discussions? Any hints?
> 

Ah okay, the earlier discussions revolve mainly around moving the regmap
from the SD/MMC driver into the clock driver. But there weren't any
issue raised for putting the regmap in the prepare function.

If you're curious, here are the links to the discussion for adding the
clk-phase to the driver:

http://archive.arm.linux.org.uk/lurker/message/20131212.203042.d37c8ee9.en.html

http://archive.arm.linux.org.uk/lurker/message/20140109.213116.1f13b27a.en.html

But perhaps putting the regmap lookup in the init function is the
correct way to do this?

Thanks,
Dinh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCHv3 2/4] clk: socfpga: add a clock driver for the Arria 10 platform

[Dinh Nguyen]

On 5/15/15 7:52 PM, Stephen Boyd wrote:
> On 05/07, dingu...@opensource.altera.com wrote:
>> diff --git a/drivers/clk/socfpga/clk-gate-a10.c 
>> b/drivers/clk/socfpga/clk-gate-a10.c
>> new file mode 100644
>> index 000..fadf6f7
>> --- /dev/null
>> +++ b/drivers/clk/socfpga/clk-gate-a10.c
>> @@ -0,0 +1,187 @@
> [...]
>> +
>> +static int socfpga_clk_prepare(struct clk_hw *hwclk)
>> +{
>> +struct socfpga_gate_clk *socfpgaclk = to_socfpga_gate_clk(hwclk);
>> +struct regmap *sys_mgr_base_addr;
>> +int i;
>> +u32 hs_timing;
>> +u32 clk_phase[2];
>> +
>> +if (socfpgaclk->clk_phase[0] || socfpgaclk->clk_phase[1]) {
>> +sys_mgr_base_addr = 
>> syscon_regmap_lookup_by_compatible("altr,sys-mgr");
>> +if (IS_ERR(sys_mgr_base_addr)) {
> 
> Is there a reason the syscon is grabbed lazily in prepare? Why
> not get it before registering this clock?

This syscon node is only associated with clocks that have a clk-phase
property, which on the SoCFPGA platform, is the SD/MMC clocks. The way
to implement this went through quite a few rounds of discussion for the
Cyclone5/Arria5 platform before settling to this method.

The reason why syscon is grabbed here is that the setting of the clock
phase must be done before enabling of the clock, so it seem that prepare
was a good place. Should this be move moved to the socfpga_gate_init()
instead?

> 
>> +pr_err("%s: failed to find altr,sys-mgr regmap!\n", 
>> __func__);
>> +return -EINVAL;
>> +}
>> +
>> +for (i = 0; i < 2; i++) {
> 
> i < ARRAY_SIZE(clk_phase) ?
> 

Will fix.

>> +switch (socfpgaclk->clk_phase[i]) {
>> +case 0:
>> +clk_phase[i] = 0;
>> +break;
>> +case 45:
>> +clk_phase[i] = 1;
>> +break;
>> +case 90:
>> +clk_phase[i] = 2;
>> +break;
>> +case 135:
>> +clk_phase[i] = 3;
>> +break;
>> +case 180:
>> +clk_phase[i] = 4;
>> +break;
>> +case 225:
>> +clk_phase[i] = 5;
>> +break;
>> +case 270:
>> +clk_phase[i] = 6;
>> +break;
>> +case 315:
>> +clk_phase[i] = 7;
>> +break;
>> +default:
>> +clk_phase[i] = 0;
>> +break;
>> +}
>> +}
>> +
>> +hs_timing = SYSMGR_SDMMC_CTRL_SET(clk_phase[0], clk_phase[1]);
>> +regmap_write(sys_mgr_base_addr, SYSMGR_SDMMCGRP_CTRL_OFFSET,
>> + hs_timing);
>> +}
>> +return 0;
>> +}
>> +
>> +static struct clk_ops gateclk_ops = {
> 
> const?
> 

I cannot make this a const as I am assigning the .enable/.disable to use
the common clk_gate_ops.

>> +.prepare = socfpga_clk_prepare,
>> +.recalc_rate = socfpga_gate_clk_recalc_rate,
>> +};
>> +
>> diff --git a/drivers/clk/socfpga/clk-periph-a10.c 
>> b/drivers/clk/socfpga/clk-periph-a10.c
>> new file mode 100644
>> index 000..81b9274
>> --- /dev/null
>> +++ b/drivers/clk/socfpga/clk-periph-a10.c
>> @@ -0,0 +1,131 @@
> [...]
>> + */
>> +#include 
> 
> Are you using this include?
> 
>> +#include 
> 
> Are you using this include?
> 
> Applies to every file added in this patch.

Removed...

> 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#include "clk.h"
>> +
>> +#define CLK_MGR_FREE_SHIFT  16
>> +#define CLK_MGR_FREE_MASK   0x7
>> +
>> +#define SOCFPGA_MPU_FREE_CLK"mpu_free_clk"
>> +#define SOCFPGA_NOC_FREE_CLK"noc_free_clk"
>> +#define SOCFPGA_SDMMC_FREE_CLK  "sdmmc_free_clk"
> [..]
>> +
>> +static __init void __socfpga_periph_init(struct device_node *node,
>> +const struct clk_ops *ops)
>> +{
> [..]
>> +init.name = clk_name;
>> +init.ops = ops;
>> +init.flags = 0;
>> +
>> +parent_name = of_clk_get_parent_name(node, 0);
>> +init.num_parents = 1;
>> +init.parent_names = _name;
>> +
>> +periph_clk->hw.hw.init = 
>> +
>> +clk = clk_register(NULL, _clk->hw.hw);
>> +if (WARN_ON(IS_ERR(clk))) {
>> +kfree(periph_clk);
>> +return;
>> +}
>> +rc = of_clk_add_provider(node, of_clk_src_simple_get, clk);
> 
> Why not check the return value?

Added check...

> 
>> +
>> diff --git a/drivers/clk/socfpga/clk-pll-a10.c 
>> b/drivers/clk/socfpga/clk-pll-a10.c
>> new file mode 100644
>> index 000..2adc2f5
>> --- /dev/null
>> +++ b/drivers/clk/socfpga/clk-pll-a10.c
> [..]
>> +
>> +static 

Re: [PATCHv3 2/4] clk: socfpga: add a clock driver for the Arria 10 platform

[Dinh Nguyen]

On 5/19/15 4:50 PM, Stephen Boyd wrote:
 On 05/19/15 09:29, Dinh Nguyen wrote:

 On 5/15/15 7:52 PM, Stephen Boyd wrote:
 On 05/07, dingu...@opensource.altera.com wrote:
 +
 +static int socfpga_clk_prepare(struct clk_hw *hwclk)
 +{
 +  struct socfpga_gate_clk *socfpgaclk = to_socfpga_gate_clk(hwclk);
 +  struct regmap *sys_mgr_base_addr;
 +  int i;
 +  u32 hs_timing;
 +  u32 clk_phase[2];
 +
 +  if (socfpgaclk-clk_phase[0] || socfpgaclk-clk_phase[1]) {
 +  sys_mgr_base_addr = 
 syscon_regmap_lookup_by_compatible(altr,sys-mgr);
 +  if (IS_ERR(sys_mgr_base_addr)) {
 Is there a reason the syscon is grabbed lazily in prepare? Why
 not get it before registering this clock?
 This syscon node is only associated with clocks that have a clk-phase
 property, which on the SoCFPGA platform, is the SD/MMC clocks. The way
 to implement this went through quite a few rounds of discussion for the
 Cyclone5/Arria5 platform before settling to this method.

 The reason why syscon is grabbed here is that the setting of the clock
 phase must be done before enabling of the clock, so it seem that prepare
 was a good place. Should this be move moved to the socfpga_gate_init()
 instead?
 
 I was expecting the regmap to be found before the clock is registered
 and stored away into the  socfpga_gate_clk structure. Getting the regmap
 during prepare is akin to ioremapping a register region during prepare,
 which doesn't sound right at all. Maybe there's some good reason in the
 earlier discussions? Any hints?
 

Ah okay, the earlier discussions revolve mainly around moving the regmap
from the SD/MMC driver into the clock driver. But there weren't any
issue raised for putting the regmap in the prepare function.

If you're curious, here are the links to the discussion for adding the
clk-phase to the driver:

http://archive.arm.linux.org.uk/lurker/message/20131212.203042.d37c8ee9.en.html

http://archive.arm.linux.org.uk/lurker/message/20140109.213116.1f13b27a.en.html

But perhaps putting the regmap lookup in the init function is the
correct way to do this?

Thanks,
Dinh
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCHv3 2/4] clk: socfpga: add a clock driver for the Arria 10 platform

[Dinh Nguyen]

On 5/15/15 7:52 PM, Stephen Boyd wrote:
 On 05/07, dingu...@opensource.altera.com wrote:
 diff --git a/drivers/clk/socfpga/clk-gate-a10.c 
 b/drivers/clk/socfpga/clk-gate-a10.c
 new file mode 100644
 index 000..fadf6f7
 --- /dev/null
 +++ b/drivers/clk/socfpga/clk-gate-a10.c
 @@ -0,0 +1,187 @@
 [...]
 +
 +static int socfpga_clk_prepare(struct clk_hw *hwclk)
 +{
 +struct socfpga_gate_clk *socfpgaclk = to_socfpga_gate_clk(hwclk);
 +struct regmap *sys_mgr_base_addr;
 +int i;
 +u32 hs_timing;
 +u32 clk_phase[2];
 +
 +if (socfpgaclk-clk_phase[0] || socfpgaclk-clk_phase[1]) {
 +sys_mgr_base_addr = 
 syscon_regmap_lookup_by_compatible(altr,sys-mgr);
 +if (IS_ERR(sys_mgr_base_addr)) {
 
 Is there a reason the syscon is grabbed lazily in prepare? Why
 not get it before registering this clock?

This syscon node is only associated with clocks that have a clk-phase
property, which on the SoCFPGA platform, is the SD/MMC clocks. The way
to implement this went through quite a few rounds of discussion for the
Cyclone5/Arria5 platform before settling to this method.

The reason why syscon is grabbed here is that the setting of the clock
phase must be done before enabling of the clock, so it seem that prepare
was a good place. Should this be move moved to the socfpga_gate_init()
instead?

 
 +pr_err(%s: failed to find altr,sys-mgr regmap!\n, 
 __func__);
 +return -EINVAL;
 +}
 +
 +for (i = 0; i  2; i++) {
 
 i  ARRAY_SIZE(clk_phase) ?
 

Will fix.

 +switch (socfpgaclk-clk_phase[i]) {
 +case 0:
 +clk_phase[i] = 0;
 +break;
 +case 45:
 +clk_phase[i] = 1;
 +break;
 +case 90:
 +clk_phase[i] = 2;
 +break;
 +case 135:
 +clk_phase[i] = 3;
 +break;
 +case 180:
 +clk_phase[i] = 4;
 +break;
 +case 225:
 +clk_phase[i] = 5;
 +break;
 +case 270:
 +clk_phase[i] = 6;
 +break;
 +case 315:
 +clk_phase[i] = 7;
 +break;
 +default:
 +clk_phase[i] = 0;
 +break;
 +}
 +}
 +
 +hs_timing = SYSMGR_SDMMC_CTRL_SET(clk_phase[0], clk_phase[1]);
 +regmap_write(sys_mgr_base_addr, SYSMGR_SDMMCGRP_CTRL_OFFSET,
 + hs_timing);
 +}
 +return 0;
 +}
 +
 +static struct clk_ops gateclk_ops = {
 
 const?
 

I cannot make this a const as I am assigning the .enable/.disable to use
the common clk_gate_ops.

 +.prepare = socfpga_clk_prepare,
 +.recalc_rate = socfpga_gate_clk_recalc_rate,
 +};
 +
 diff --git a/drivers/clk/socfpga/clk-periph-a10.c 
 b/drivers/clk/socfpga/clk-periph-a10.c
 new file mode 100644
 index 000..81b9274
 --- /dev/null
 +++ b/drivers/clk/socfpga/clk-periph-a10.c
 @@ -0,0 +1,131 @@
 [...]
 + */
 +#include linux/clk.h
 
 Are you using this include?
 
 +#include linux/clkdev.h
 
 Are you using this include?
 
 Applies to every file added in this patch.

Removed...

 
 +#include linux/clk-provider.h
 +#include linux/io.h
 +#include linux/of.h
 +
 +#include clk.h
 +
 +#define CLK_MGR_FREE_SHIFT  16
 +#define CLK_MGR_FREE_MASK   0x7
 +
 +#define SOCFPGA_MPU_FREE_CLKmpu_free_clk
 +#define SOCFPGA_NOC_FREE_CLKnoc_free_clk
 +#define SOCFPGA_SDMMC_FREE_CLK  sdmmc_free_clk
 [..]
 +
 +static __init void __socfpga_periph_init(struct device_node *node,
 +const struct clk_ops *ops)
 +{
 [..]
 +init.name = clk_name;
 +init.ops = ops;
 +init.flags = 0;
 +
 +parent_name = of_clk_get_parent_name(node, 0);
 +init.num_parents = 1;
 +init.parent_names = parent_name;
 +
 +periph_clk-hw.hw.init = init;
 +
 +clk = clk_register(NULL, periph_clk-hw.hw);
 +if (WARN_ON(IS_ERR(clk))) {
 +kfree(periph_clk);
 +return;
 +}
 +rc = of_clk_add_provider(node, of_clk_src_simple_get, clk);
 
 Why not check the return value?

Added check...

 
 +
 diff --git a/drivers/clk/socfpga/clk-pll-a10.c 
 b/drivers/clk/socfpga/clk-pll-a10.c
 new file mode 100644
 index 000..2adc2f5
 --- /dev/null
 +++ b/drivers/clk/socfpga/clk-pll-a10.c
 [..]
 +
 +static u8 clk_pll_get_parent(struct clk_hw *hwclk)
 +{
 +struct socfpga_pll *socfpgaclk = to_socfpga_clk(hwclk);
 +u32 pll_src;
 +
 +pll_src = readl(socfpgaclk-hw.reg);
 +
 +return (pll_src  

Re: [PATCH 3/4] edac, altera: Addition of Arria10 EDAC

[Dinh Nguyen]

On 05/13/2015 04:49 PM, ttha...@opensource.altera.com wrote:
> From: Thor Thayer 
> 
> The Arria10 SDRAM and ECC system differs significantly from the
> Cyclone5 and Arria5 SoCs. This patch adds support for the Arria10
> SoC.
> 1) IRQ handler needs to support SHARED IRQ
> 2) Support sberr and dberr address reporting.
> 
> Signed-off-by: Thor Thayer 
> ---
>  drivers/edac/altera_edac.c |  132 
> ++--
>  drivers/edac/altera_edac.h |   85 
>  2 files changed, 201 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c
> index 204ad2d..735a180 100644
> --- a/drivers/edac/altera_edac.c
> +++ b/drivers/edac/altera_edac.c
> @@ -42,6 +42,7 @@ const struct altr_sdram_prv_data c5_data = {
>   .ecc_stat_ce_mask   = CV_DRAMSTS_SBEERR,
>   .ecc_stat_ue_mask   = CV_DRAMSTS_DBEERR,
>   .ecc_saddr_offset   = CV_ERRADDR_OFST,
> + .ecc_daddr_offset   = CV_ERRADDR_OFST,
>   .ecc_cecnt_offset   = CV_SBECOUNT_OFST,
>   .ecc_uecnt_offset   = CV_DBECOUNT_OFST,
>   .ecc_irq_en_offset  = CV_DRAMINTR_OFST,
> @@ -57,37 +58,62 @@ const struct altr_sdram_prv_data c5_data = {
>  #endif
>  };
>  
> +const struct altr_sdram_prv_data a10_data = {\

This should be static.

> + .ecc_ctrl_offset= A10_ECCCTRL1_OFST,
> + .ecc_ctl_en_mask= A10_ECCCTRL1_ECC_EN,
> + .ecc_stat_offset= A10_INTSTAT_OFST,
> + .ecc_stat_ce_mask   = A10_INTSTAT_SBEERR,
> + .ecc_stat_ue_mask   = A10_INTSTAT_DBEERR,
> + .ecc_saddr_offset   = A10_SERRADDR_OFST,
> + .ecc_daddr_offset   = A10_DERRADDR_OFST,
> + .ecc_irq_en_offset  = A10_ERRINTEN_OFST,
> + .ecc_irq_en_mask= A10_ECC_IRQ_EN_MASK,
> + .ecc_irq_clr_offset = A10_INTSTAT_OFST,
> + .ecc_irq_clr_mask   = (A10_INTSTAT_SBEERR | A10_INTSTAT_DBEERR),
> + .ecc_cnt_rst_offset = A10_ECCCTRL1_OFST,
> + .ecc_cnt_rst_mask   = A10_ECC_CNT_RESET_MASK,
> +#ifdef CONFIG_EDAC_DEBUG
> + .ce_ue_trgr_offset  = A10_DIAGINTTEST_OFST,
> + .ce_set_mask= A10_DIAGINT_TSERRA_MASK,
> + .ue_set_mask= A10_DIAGINT_TDERRA_MASK,
> +#endif
> +};
> +
>


> +
>  static int altr_sdram_probe(struct platform_device *pdev)
>  {
>   const struct of_device_id *id;
> @@ -221,8 +295,8 @@ static int altr_sdram_probe(struct platform_device *pdev)
>   struct regmap *mc_vbase;
>   struct dimm_info *dimm;
>   u32 read_reg;
> - int irq, res = 0;
> - unsigned long mem_size;
> + int irq, irq2, res = 0;
> + unsigned long mem_size, irqflags;
>  
>   id = of_match_device(altr_sdram_ctrl_of_match, >dev);
>   if (!id)
> @@ -288,6 +362,9 @@ static int altr_sdram_probe(struct platform_device *pdev)
>   return -ENODEV;
>   }
>  
> + /* Arria10 has a 2nd IRQ */
> + irq2 = platform_get_irq(pdev, 1);
> +
>   layers[0].type = EDAC_MC_LAYER_CHIP_SELECT;
>   layers[0].size = 1;
>   layers[0].is_virt_csrow = true;
> @@ -332,8 +409,31 @@ static int altr_sdram_probe(struct platform_device *pdev)
>   if (res < 0)
>   goto err;
>  
> + /* Only the Arria10 has separate IRQs */
> + if (irq2 > 0) {
> + /* Arria10 specific initialization */
> + res = a10_init(mc_vbase);
> + if (res < 0)
> + goto err2;
> +
> + res = a10_unmask_irq(pdev, A10_DDR0_IRQ_MASK);
> + if (res < 0)
> + goto err2;
> +
> + res = devm_request_irq(>dev, irq2,
> +altr_sdram_mc_err_handler,
> +IRQF_SHARED, dev_name(>dev), mci);
> + if (res < 0) {
> + edac_mc_printk(mci, KERN_ERR,
> +"Unable to request irq %d\n", irq2);
> + res = -ENODEV;
> + goto err2;
> + }
> + irqflags = IRQF_SHARED;
> + }
> +
>   res = devm_request_irq(>dev, irq, altr_sdram_mc_err_handler,
> -0, dev_name(>dev), mci);
> +irqflags, dev_name(>dev), mci);

irqflags was never set for the case of !(irq2 > 0).

Dinh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/