Re: [PATCH net-next] net: phy: phy_support_sym_pause: Clear Asym Pause

[LABBE Corentin]

On Sat, Oct 20, 2018 at 10:41:28PM +0200, Andrew Lunn wrote:
> When indicating the MAC supports Symmetric Pause, clear the Asymmetric
> Pause bit, which could of been already set is the PHY supports it.
> 
> Reported-by: Labbe Corentin 
> Fixes: c306ad36184f ("net: ethernet: Add helper for MACs which support pause")
> Signed-off-by: Andrew Lunn 
> ---
>  drivers/net/phy/phy_device.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index 43cb08dcce81..ab33d1777132 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -1940,6 +1940,7 @@ EXPORT_SYMBOL(phy_remove_link_mode);
>   */
>  void phy_support_sym_pause(struct phy_device *phydev)
>  {
> + phydev->supported &= ~SUPPORTED_Asym_Pause;
>   phydev->supported |= SUPPORTED_Pause;
>   phydev->advertising = phydev->supported;
>  }
> -- 
> 2.19.0
> 

Thanks, it made my imx6q-sabrelite works again with next-20181019.
Tested-by: Corentin Labbe 

For completeness, this is the ethtool output which confirm it.
ethtool version 4.16
Settings for eth0:
Supported ports: [ TP MII ]
Supported link modes:   10baseT/Half 10baseT/Full 
100baseT/Half 100baseT/Full 
1000baseT/Full 
Supported pause frame use: Symmetric
Supports auto-negotiation: Yes
Supported FEC modes: Not reported
Advertised link modes:  10baseT/Half 10baseT/Full 
100baseT/Half 100baseT/Full 
1000baseT/Full 
Advertised pause frame use: Symmetric
Advertised auto-negotiation: Yes
Advertised FEC modes: Not reported
Link partner advertised link modes:  10baseT/Half 10baseT/Full 
 100baseT/Half 100baseT/Full 
 1000baseT/Half 1000baseT/Full 
Link partner advertised pause frame use: Symmetric Receive-only
Link partner advertised auto-negotiation: Yes
Link partner advertised FEC modes: Not reported
Speed: 1000Mb/s
Duplex: Full
Port: MII
PHYAD: 6
Transceiver: internal
Auto-negotiation: on
Supports Wake-on: d
Wake-on: d
Link detected: yes

Re: [PATCH] net: ethernet: fec: Add missing SPEED_

[LABBE Corentin]

On Thu, Oct 18, 2018 at 11:55:49AM -0700, Florian Fainelli wrote:
> On 10/18/2018 11:47 AM, LABBE Corentin wrote:
> > On Thu, Oct 18, 2018 at 11:39:24AM -0700, Florian Fainelli wrote:
> >> On 10/18/2018 08:05 AM, Corentin Labbe wrote:
> >>> Since commit 58056c1e1b0e ("net: ethernet: Use phy_set_max_speed() to 
> >>> limit advertised speed"), the fec driver is unable to get any link.
> >>> This is due to missing SPEED_.
> >>
> >> But SPEED_1000 is defined in include/uapi/linux/ethtool.h as 1000, so
> >> surely this would amount to the same code paths being taken or am I
> >> missing something here?
> > 
> > The bisect session pointed your patch, reverting it fix the issue.
> > BUT since the fix seemed trivial I sent the patch without more test then 
> > compile it.
> > Sorry, I have just found some minutes ago that it didnt fix the issue.
> > 
> > But your patch is still the cause for sure.
> > 
> 
> What you are writing is really lowering the confidence level, first
> Andrew is the author of that patch, and second "just compiling" and
> pretending this fixes a problem when it does not is not quite what I
> would expect.
> 
> I don't have a problem helping you find the solution or the right fix
> though, even if it is not my patch, but please get the author and actual
> problem right so we can move forward in confidence, thanks!

Sorry again, I wanted to acknoledge my error but I did it too fast and late.
And sorry to have confound you with Andrew.

Re: [PATCH v4 01/10] ethernet: add sun8i-emac driver

[LABBE Corentin]

On Fri, Oct 07, 2016 at 08:02:39AM -0700, Joe Perches wrote:
> On Fri, 2016-10-07 at 10:25 +0200, Corentin Labbe wrote:
> > This patch add support for sun8i-emac ethernet MAC hardware.
> > It could be found in Allwinner H3/A83T/A64 SoCs.
> 
> trivial notes:
> 
> > diff --git a/drivers/net/ethernet/allwinner/sun8i-emac.c 
> > b/drivers/net/ethernet/allwinner/sun8i-emac.c
> []
> > +static const char const estats_str[][ETH_GSTRING_LEN] = {
> 
> one too many const
> 
> > +/* MAGIC value for knowing if a descriptor is available or not */
> > +#define DCLEAN cpu_to_le32(BIT(16) | BIT(14) | BIT(12) | BIT(10) | BIT(9))
> 
> Aren't there #defines for these bits?
> 
> > +static void sun8i_emac_flow_ctrl(struct sun8i_emac_priv *priv, int duplex,
> > +int fc)
> > +{
> > +   u32 flow = 0;
> > +
> > +   flow = readl(priv->base + EMAC_RX_CTL0);
> > +   if (fc & EMAC_FLOW_RX)
> > +   flow |= BIT(16);
> > +   else
> > +   flow &= ~BIT(16);
> > +   writel(flow, priv->base + EMAC_RX_CTL0);
> > +
> > +   flow = readl(priv->base + EMAC_TX_FLOW_CTL);
> > +   if (fc & EMAC_FLOW_TX)
> > +   flow |= BIT(0);
> > +   else
> > +   flow &= ~BIT(0);
> 
> more magic bits that could be #defines
> 
> > +static int sun8i_emac_rx_from_ddesc(struct net_device *ndev, int i)
> > +{
> > []
> > +   /* the checksum or length of received frame's payload is wrong*/
> > +   if (dstatus & BIT(0)) {
> []
> > +   if (dstatus & BIT(1)) {
> []
> > +   if ((dstatus & BIT(3))) {
> 
> etc...

Thanks for the review, I will fix all thoses issue.

Regards
Corentin Labbe

Re: [PATCH v4 03/10] ARM: sun8i: dt: Add DT bindings documentation for Allwinner sun8i-emac

[LABBE Corentin]

On Mon, Oct 10, 2016 at 10:13:35AM -0500, Rob Herring wrote:
> On Fri, Oct 07, 2016 at 10:25:50AM +0200, Corentin Labbe wrote:
> > This patch adds documentation for Device-Tree bindings for the
> > Allwinner sun8i-emac driver.
> > 
> > Signed-off-by: Corentin Labbe 
> > ---
> >  .../bindings/net/allwinner,sun8i-emac.txt  | 70 
> > ++
> >  1 file changed, 70 insertions(+)
> >  create mode 100644 
> > Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt 
> > b/Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt
> > new file mode 100644
> > index 000..92e4ef3b
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt
> > @@ -0,0 +1,70 @@
> > +* Allwinner sun8i EMAC ethernet controller
> > +
> > +Required properties:
> > +- compatible: should be one of the following string:
> > +   "allwinner,sun8i-a83t-emac"
> > +   "allwinner,sun8i-h3-emac"
> > +   "allwinner,sun50i-a64-emac"
> > +- reg: address and length of the register for the device.
> > +- syscon: A phandle to the syscon of the SoC
> > +- interrupts: interrupt for the device
> > +- clocks: A phandle to the reference clock for this device
> > +- clock-names: should be "ahb"
> > +- resets: A phandle to the reset control for this device
> > +- reset-names: should be "ahb"
> > +- phy-mode: See ethernet.txt
> > +- phy-handle: See ethernet.txt
> > +- #address-cells: shall be 1
> > +- #size-cells: shall be 0
> > +
> > +Optional properties:
> > +- allwinner,tx-delay: TX clock delay chain value. Range value is 0-0x07. 
> > Default is 0)
> > +- allwinner,rx-delay: RX clock delay chain value. Range value is 0-0x1F. 
> > Default is 0)
> > +Both delay properties does not have units, there are arbitrary value.
> 
> They have to have some sort of units. Some number of clocks perhaps. Or 
> just say what register field they correspond to.
> 

I have re-read all 3 datasheets (A64/H3/A83T) and made string search for 
finding any information.
But still found nothing, no unit, no more informations than I already wrote in 
this file.

For the register field, just saying that it is used in the syscon register 
EMAC_CLK_REG is sufficient ?

> > +The TX/RX clock delay chain settings are board specific and could be found
> > +in vendor FEX files.
> > +

Regards

Corentin Labbe

Re: [PATCH v4 10/10] ARM: sunxi: Enable sun8i-emac driver on multi_v7_defconfig

[LABBE Corentin]

On Tue, Oct 11, 2016 at 11:40:42AM +0200, Maxime Ripard wrote:
> On Mon, Oct 10, 2016 at 03:09:43PM +0200, Jean-Francois Moine wrote:
> > On Mon, 10 Oct 2016 14:35:11 +0200
> > LABBE Corentin <clabbe.montj...@gmail.com> wrote:
> > 
> > > On Mon, Oct 10, 2016 at 02:30:46PM +0200, Maxime Ripard wrote:
> > > > On Fri, Oct 07, 2016 at 10:25:57AM +0200, Corentin Labbe wrote:
> > > > > Enable the sun8i-emac driver in the multi_v7 default configuration
> > > > > 
> > > > > Signed-off-by: Corentin Labbe <clabbe.montj...@gmail.com>
> > > > > ---
> > > > >  arch/arm/configs/multi_v7_defconfig | 1 +
> > > > >  1 file changed, 1 insertion(+)
> > > > > 
> > > > > diff --git a/arch/arm/configs/multi_v7_defconfig 
> > > > > b/arch/arm/configs/multi_v7_defconfig
> > > > > index 5845910..f44d633 100644
> > > > > --- a/arch/arm/configs/multi_v7_defconfig
> > > > > +++ b/arch/arm/configs/multi_v7_defconfig
> > > > > @@ -229,6 +229,7 @@ CONFIG_NETDEVICES=y
> > > > >  CONFIG_VIRTIO_NET=y
> > > > >  CONFIG_HIX5HD2_GMAC=y
> > > > >  CONFIG_SUN4I_EMAC=y
> > > > > +CONFIG_SUN8I_EMAC=y
> > > > 
> > > > Any reason to build it statically?
> > > 
> > > No, just copied the same than CONFIG_SUN4I_EMAC that probably do
> > > not need it also.
> > 
> > All arm configs are done the same way, and, some day, the generic ARM
> > V7 kernel will not be loadable in 1Gb RAM...
> 
> Yeah, if possible, I'd really like to avoid introducing statically
> built drivers to multi_v7.
> 

I forgot to said it in my first answer, but yes I will change it.

Regards

Re: [PATCH v4 10/10] ARM: sunxi: Enable sun8i-emac driver on multi_v7_defconfig

[LABBE Corentin]

On Mon, Oct 10, 2016 at 02:30:46PM +0200, Maxime Ripard wrote:
> On Fri, Oct 07, 2016 at 10:25:57AM +0200, Corentin Labbe wrote:
> > Enable the sun8i-emac driver in the multi_v7 default configuration
> > 
> > Signed-off-by: Corentin Labbe 
> > ---
> >  arch/arm/configs/multi_v7_defconfig | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/arch/arm/configs/multi_v7_defconfig 
> > b/arch/arm/configs/multi_v7_defconfig
> > index 5845910..f44d633 100644
> > --- a/arch/arm/configs/multi_v7_defconfig
> > +++ b/arch/arm/configs/multi_v7_defconfig
> > @@ -229,6 +229,7 @@ CONFIG_NETDEVICES=y
> >  CONFIG_VIRTIO_NET=y
> >  CONFIG_HIX5HD2_GMAC=y
> >  CONFIG_SUN4I_EMAC=y
> > +CONFIG_SUN8I_EMAC=y
> 
> Any reason to build it statically?
> 

No, just copied the same than CONFIG_SUN4I_EMAC that probably do not need it 
also.

Regards

Corentin Labbe

Re: [RFC PATCH 9/9] ethernet: sun8i-emac: add pm_runtime support

[LABBE Corentin]

On Mon, Sep 12, 2016 at 10:44:51PM +0200, Maxime Ripard wrote:
> Hi,
> 
> On Fri, Sep 09, 2016 at 02:45:17PM +0200, Corentin Labbe wrote:
> > This patch add pm_runtime support to sun8i-emac.
> > For the moment, only basic support is added, (the device is marked as
> > used when net/open)
> > 
> > Signed-off-by: Corentin Labbe 
> > ---
> >  drivers/net/ethernet/allwinner/sun8i-emac.c | 62 
> > -
> >  1 file changed, 60 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/allwinner/sun8i-emac.c 
> > b/drivers/net/ethernet/allwinner/sun8i-emac.c
> > index 1c4bc80..cce886e 100644
> > --- a/drivers/net/ethernet/allwinner/sun8i-emac.c
> > +++ b/drivers/net/ethernet/allwinner/sun8i-emac.c
> > @@ -9,7 +9,6 @@
> >   * - MAC filtering
> >   * - Jumbo frame
> >   * - features rx-all (NETIF_F_RXALL_BIT)
> > - * - PM runtime
> >   */
> >  #include 
> >  #include 
> > @@ -27,6 +26,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> > @@ -1301,11 +1301,18 @@ static int sun8i_emac_open(struct net_device *ndev)
> > int err;
> > u32 v;
> >  
> > +   err = pm_runtime_get_sync(priv->dev);
> > +   if (err) {
> > +   pm_runtime_put_noidle(priv->dev);
> > +   dev_err(priv->dev, "pm_runtime error: %d\n", err);
> > +   return err;
> > +   }
> > +
> > err = request_irq(priv->irq, sun8i_emac_dma_interrupt, 0,
> >   dev_name(priv->dev), ndev);
> > if (err) {
> > dev_err(priv->dev, "Cannot request IRQ: %d\n", err);
> > -   return err;
> > +   goto err_runtime;
> > }
> >  
> > /* Set interface mode (and configure internal PHY on H3) */
> > @@ -1395,6 +1402,8 @@ err_syscon:
> > sun8i_emac_unset_syscon(ndev);
> >  err_irq:
> > free_irq(priv->irq, ndev);
> > +err_runtime:
> > +   pm_runtime_put(priv->dev);
> > return err;
> >  }
> >  
> > @@ -1483,6 +1492,8 @@ static int sun8i_emac_stop(struct net_device *ndev)
> > dma_free_coherent(priv->dev, priv->nbdesc_tx * sizeof(struct dma_desc),
> >   priv->dd_tx, priv->dd_tx_phy);
> >  
> > +   pm_runtime_put(priv->dev);
> > +
> > return 0;
> >  }
> >  
> > @@ -2210,6 +2221,8 @@ static int sun8i_emac_probe(struct platform_device 
> > *pdev)
> > goto probe_err;
> > }
> >  
> > +   pm_runtime_enable(priv->dev);
> > +
> > return 0;
> >  
> >  probe_err:
> > @@ -2221,6 +2234,8 @@ static int sun8i_emac_remove(struct platform_device 
> > *pdev)
> >  {
> > struct net_device *ndev = platform_get_drvdata(pdev);
> >  
> > +   pm_runtime_disable(>dev);
> > +
> > unregister_netdev(ndev);
> > platform_set_drvdata(pdev, NULL);
> > free_netdev(ndev);
> > @@ -2228,6 +2243,47 @@ static int sun8i_emac_remove(struct platform_device 
> > *pdev)
> > return 0;
> >  }
> >  
> > +static int __maybe_unused sun8i_emac_suspend(struct platform_device *pdev, 
> > pm_message_t state)
> > +{
> > +   struct net_device *ndev = platform_get_drvdata(pdev);
> > +   struct sun8i_emac_priv *priv = netdev_priv(ndev);
> > +
> > +   napi_disable(>napi);
> > +
> > +   if (netif_running(ndev))
> > +   netif_device_detach(ndev);
> > +
> > +   sun8i_emac_stop_tx(ndev);
> > +   sun8i_emac_stop_rx(ndev);
> > +
> > +   sun8i_emac_rx_clean(ndev);
> > +   sun8i_emac_tx_clean(ndev);
> > +
> > +   phy_stop(ndev->phydev);
> > +
> > +   return 0;
> > +}
> > +
> > +static int __maybe_unused sun8i_emac_resume(struct platform_device *pdev)
> > +{
> > +   struct net_device *ndev = platform_get_drvdata(pdev);
> > +   struct sun8i_emac_priv *priv = netdev_priv(ndev);
> > +
> > +   phy_start(ndev->phydev);
> > +
> > +   sun8i_emac_start_tx(ndev);
> > +   sun8i_emac_start_rx(ndev);
> > +
> > +   if (netif_running(ndev))
> > +   netif_device_attach(ndev);
> > +
> > +   netif_start_queue(ndev);
> > +
> > +   napi_enable(>napi);
> > +
> > +   return 0;
> > +}
> 
> The main idea behind the runtime PM hooks is that they bring the
> device to a working state and shuts it down when it's not needed
> anymore.
> 

I expect that the first part (all pm_runtime_xxx) of the patch bring that.
When the interface is not opened:
cat /sys/devices/platform/soc/1c3.ethernet/power/runtime_status 
suspended

> However, they shouldn't be called when the device is still in used, so
> all the mangling with NAPI, the phy and so on is irrelevant here, but
> the clocks, resets, for example, are.
> 

I do the same as other ethernet driver for suspend/resume.

> >  static const struct of_device_id sun8i_emac_of_match_table[] = {
> > { .compatible = "allwinner,sun8i-a83t-emac",
> >   .data = _variant_a83t },
> > @@ -2246,6 +2302,8 @@ static struct platform_driver sun8i_emac_driver = {
> > .name   = "sun8i-emac",
> > .of_match_table = sun8i_emac_of_match_table,
> > },
> > +   .suspend= sun8i_emac_suspend,
> > +   

Re: [PATCH v3 5/9] ARM: dts: sun8i-h3: add sun8i-emac ethernet driver

[LABBE Corentin]

On Mon, Sep 12, 2016 at 09:29:33AM +0200, Maxime Ripard wrote:
> On Fri, Sep 09, 2016 at 02:45:13PM +0200, Corentin Labbe wrote:
> > The sun8i-emac is an ethernet MAC hardware that support 10/100/1000
> > speed.
> > 
> > This patch enable the sun8i-emac on the Allwinner H3 SoC Device-tree.
> > The SoC H3 have an internal PHY, so optionals syscon and ephy are set.
> > 
> > Signed-off-by: Corentin Labbe 
> > ---
> >  arch/arm/boot/dts/sun8i-h3.dtsi | 19 +++
> >  1 file changed, 19 insertions(+)
> > 
> > diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi 
> > b/arch/arm/boot/dts/sun8i-h3.dtsi
> > index a39da6f..a3ac476 100644
> > --- a/arch/arm/boot/dts/sun8i-h3.dtsi
> > +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
> > @@ -50,6 +50,10 @@
> >  / {
> > interrupt-parent = <>;
> >  
> > +   aliases {
> > +   ethernet0 = 
> > +   };
> > +
> 
> This needs to be done at the board level.
> 

ok

> > cpus {
> > #address-cells = <1>;
> > #size-cells = <0>;
> > @@ -446,6 +450,21 @@
> > status = "disabled";
> > };
> >  
> > +   emac: ethernet@1c3 {
> > +   compatible = "allwinner,sun8i-h3-emac";
> > +   syscon = <>;
> > +   reg = <0x01c3 0x104>;
> > +   reg-names = "emac";
> 
> You don't need reg-names anymore.
> 

ok

> > +   interrupts = ;
> > +   resets = < RST_BUS_EMAC>, < RST_BUS_EPHY>;
> > +   reset-names = "ahb", "ephy";
> > +   clocks = < CLK_BUS_EMAC>, < CLK_BUS_EPHY>;
> > +   clock-names = "ahb", "ephy";
> 
> I still believe that having the same node for both the PHY and the MAC
> is wrong.
> 

Ok I have moved clock/reset of ephy in its node.

Thanks

Regards

Corentin Labbe

Re: [PATCH v3 8/9] ARM: sunxi: Enable sun8i-emac driver on sunxi_defconfig

[LABBE Corentin]

On Mon, Sep 12, 2016 at 09:30:08AM +0200, Maxime Ripard wrote:
> Hi,
> 
> On Fri, Sep 09, 2016 at 02:45:16PM +0200, Corentin Labbe wrote:
> > Enable the sun8i-emac driver in the sunxi default configuration
> > 
> > Signed-off-by: Corentin Labbe 
> 
> Could you make the same patch for multi_v7 ?
> 

I will

Thanks

Re: [PATCH v3 4/9] ARM: dts: sun8i-h3: Add dt node for the syscon control module

[LABBE Corentin]

On Mon, Sep 12, 2016 at 09:28:12AM +0200, Maxime Ripard wrote:
> On Fri, Sep 09, 2016 at 02:45:12PM +0200, Corentin Labbe wrote:
> > This patch add the dt node for the syscon register present on the
> > Allwinner H3.
> > 
> > Only two register are present in this syscon and the only one useful is
> > the one dedicated to EMAC clock.
> > 
> > Signed-off-by: Corentin Labbe 
> > ---
> >  arch/arm/boot/dts/sun8i-h3.dtsi | 5 +
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi 
> > b/arch/arm/boot/dts/sun8i-h3.dtsi
> > index fdf9fdb..a39da6f 100644
> > --- a/arch/arm/boot/dts/sun8i-h3.dtsi
> > +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
> > @@ -140,6 +140,11 @@
> > #size-cells = <1>;
> > ranges;
> >  
> > +   syscon: syscon@01c0 {
> > +   compatible = "syscon";
> 
> Having our compatible would be nice here. syscon doesn't mean anything
> by itself.
> 

Since no driver handle it, I follow what I saw in other DT.
At your choice, I can add a sun8i-syscon, but it will be unused.

> > +   reg = <0x01c0 0x34>;
> 
> And the size of our system controller is 0x1000
> 

I put the real size used, but I can put what datasheet said.

Regards

Corentin Labbe

Re: [PATCH v3 1/9] ethernet: add sun8i-emac driver

[LABBE Corentin]

On Fri, Sep 09, 2016 at 04:15:27PM +0200, Andrew Lunn wrote:
> Hi Corentin
> 
> > +static int sun8i_emac_mdio_register(struct net_device *ndev)
> > +{
> > +   struct sun8i_emac_priv *priv = netdev_priv(ndev);
> > +   struct mii_bus *bus;
> > +   int ret;
> > +
> > +   bus = mdiobus_alloc();
> 
> You can use devm_mdiobus_alloc() which will simplify your error
> handling and unregister code.
> 
>Andrew

Hello

Since the mdio bus is allocated on ndev/open, it need to be removed when 
ndev/stop is called.
So devm_mdiobus_alloc cannot be used.

Regards

Corentin Labbe

Re: [PATCH v3 3/9] ARM: sun8i: dt: Add DT bindings documentation for Allwinner sun8i-emac

[LABBE Corentin]

On Fri, Sep 09, 2016 at 04:17:10PM +0200, Andrew Lunn wrote:
> > +Optional properties:
> > +- allwinner,tx-delay: TX clock delay chain value. Range value is 0-0x07. 
> > Default is 0)
> > +- allwinner,rx-delay: RX clock delay chain value. Range value is 0-0x1F. 
> > Default is 0)
> 
> What are the units? pS? nS?
> 
>  Andrew

No units, only raw number.
I will add a comment for this.

Regards

Corentin Labbe

Re: [PATCH v3 3/9] ARM: sun8i: dt: Add DT bindings documentation for Allwinner sun8i-emac

[LABBE Corentin]

On Fri, Sep 09, 2016 at 04:04:13PM +0200, Andrew Lunn wrote:
> > +The device node referenced by "phy" or "phy-handle" should be a child node
> > +of this node. See phy.txt for the generic PHY bindings.
> 
> I've not looked at the code yet, but is this really true? Generally
> there is not this limitation. You can point to any Ethernet phy
> anyway, so long as it is on am MDIO bus.
> 
> > +
> > +Optional properties:
> > +- allwinner,tx-delay: TX clock delay chain value. Range value is 0-0x07. 
> > Default is 0)
> > +- allwinner,rx-delay: RX clock delay chain value. Range value is 0-0x1F. 
> > Default is 0)
> > +
> > +The TX/RX clock delay chain settings are board specific.
> > +
> > +Optional properties for "allwinner,sun8i-h3-emac":
> > +- allwinner,leds-active-low: EPHY LEDs are active low
> > +
> > +Example:
> > +
> > +emac: ethernet@01c0b000 {
> > +   compatible = "allwinner,sun8i-h3-emac";
> > +   syscon = <>;
> > +   reg = <0x01c0b000 0x104>;
> > +   reg-names = "emac";
> > +   interrupts = ;
> > +   resets = < RST_BUS_EMAC>, << RST_BUS_EPHY>;
> > +   reset-names = "ahb", "ephy";
> > +   clocks = < CLK_BUS_EMAC>, < CLK_BUS_EPHY>;
> > +   clock-names = "ahb", "ephy";
> > +   #address-cells = <1>;
> > +   #size-cells = <0>;
> > +
> > +   phy = <>;
> 
> ethernet.txt say:
> 
> - phy: the same as "phy-handle" property, not recommended for new bindings.
> 
> This is a new binding, please don't support it.
> 
> > +   phy-mode = "mii";
> > +   allwinner,leds-active-low;
> > +
> > +   phy1: ethernet-phy@1 {
> > +   reg = <1>;
> > +   };
> 
> It is normal to place these phy nodes inside an container node called
> mdio.
> 

Hello

Since the MDIO bus is a part of the sun8i-emac, does I really need to create 
such a mdio node ?
All example I found are mdio bus with separate driver. (others driver have the 
phy directly in [eg]mac node.

Anyway I try the following patch to solve your comments, but it breaks the PHY 
finding(Could not attach to PHY).

Regards

-->8--
--- a/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
+++ b/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
@@ -166,14 +166,18 @@
status = "okay";
 };
 
+ {
+   reg = <1>;
+   phy1: ethernet-phy@1 {
+   reg = <1>;
+   };
+};
+
  {
-   phy = <>;
+   phy-handle = <>;
phy-mode = "mii";
allwinner,leds-active-low;
status = "okay";
-   phy1: ethernet-phy@1 {
-   reg = <1>;
-   };
 };/arch/arm/boot/dts/sun8i-h3.dtsi
+++ b/arch/arm/boot/dts/sun8i-h3.dtsi
@@ -474,6 +474,11 @@
#address-cells = <1>;
#size-cells = <0>;
status = "disabled";
+
+   mdio: mdio@0 {
+   #address-cells = <1>;
+   #size-cells = <0>;
+   };
};
 
crypto: crypto@1c15000 {
--- a/drivers/net/ethernet/allwinner/sun8i-emac.c
+++ b/drivers/net/ethernet/allwinner/sun8i-emac.c
@@ -2122,7 +2122,7 @@ static int sun8i_emac_probe(struct platform_device *pdev)
return -EINVAL;
}
 
-   priv->phy_node = of_parse_phandle(node, "phy", 0);
+   priv->phy_node = of_parse_phandle(node, "phy-handle", 0);
if (!priv->phy_node) {
netdev_err(ndev, "No associated PHY\n");
return -ENODEV;

 
  {

Re: [PATCH v2 1/5] ethernet: add sun8i-emac driver

[LABBE Corentin]

> > +/* Set Management Data Clock, must be call after device reset */
> > +static void sun8i_emac_set_mdc(struct net_device *ndev)
> > +{
> > +   struct sun8i_emac_priv *priv = netdev_priv(ndev);
> > +   unsigned long rate;
> > +   u32 reg;
> > +
> > +   rate = clk_get_rate(priv->ahb_clk);
> > +   if (rate > 16000)
> > +   reg = 0x3 << 20; /* AHB / 128 */
> > +   else if (rate > 8000)
> > +   reg = 0x2 << 20; /* AHB / 64 */
> > +   else if (rate > 4000)
> > +   reg = 0x1 << 20; /* AHB / 32 */
> > +   else
> > +   reg = 0x0 << 20; /* AHB / 16 */
> > +   netif_dbg(priv, link, ndev, "MDC auto : %x\n", reg);
> > +   writel(reg, priv->base + SUN8I_EMAC_MDIO_CMD);
> 
> You could also expose that as a clock.
> 

For which purpose ?
No ethernet driver expose the MDC as clock and I dont see any interest:
- I dont think that tuning it give any gain
- Knowing it's value is of little interest

Regards

[PATCH] atm: fore200e: Do not drop const qualifier

[LABBE Corentin]

The data member of structure firmware is const and this constness is
dropped by some cast.
This patch add some const for keeping the const information.

Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
---
 drivers/atm/fore200e.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/atm/fore200e.c b/drivers/atm/fore200e.c
index 75dde90..81aaa50 100644
--- a/drivers/atm/fore200e.c
+++ b/drivers/atm/fore200e.c
@@ -2489,7 +2489,7 @@ static int fore200e_load_and_start_fw(struct fore200e 
*fore200e)
 {
 const struct firmware *firmware;
 struct device *device;
-struct fw_header *fw_header;
+const struct fw_header *fw_header;
 const __le32 *fw_data;
 u32 fw_size;
 u32 __iomem *load_addr;
@@ -2511,9 +2511,9 @@ static int fore200e_load_and_start_fw(struct fore200e 
*fore200e)
return err;
 }
 
-fw_data = (__le32 *) firmware->data;
+fw_data = (const __le32 *)firmware->data;
 fw_size = firmware->size / sizeof(u32);
-fw_header = (struct fw_header *) firmware->data;
+fw_header = (const struct fw_header *)firmware->data;
 load_addr = fore200e->virt_base + le32_to_cpu(fw_header->load_offset);
 
 DPRINTK(2, "device %s firmware being loaded at 0x%p (%d words)\n",
-- 
2.7.3

[PATCH] net: bfin_mac: Fix a few spelling fixes

[LABBE Corentin]

This patch respell some word badly spelled.
- Invidate instead of Invalidate
- proble instead of probe

Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
---
 drivers/net/ethernet/adi/bfin_mac.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/adi/bfin_mac.c 
b/drivers/net/ethernet/adi/bfin_mac.c
index 38eaea1..00f9ee3 100644
--- a/drivers/net/ethernet/adi/bfin_mac.c
+++ b/drivers/net/ethernet/adi/bfin_mac.c
@@ -192,8 +192,8 @@ static int desc_list_init(struct net_device *dev)
goto init_error;
 
skb_reserve(new_skb, NET_IP_ALIGN);
-   /* Invidate the data cache of skb->data range when it is write 
back
-* cache. It will prevent overwritting the new data from DMA
+   /* Invalidate the data cache of skb->data range when it is 
write back
+* cache. It will prevent overwriting the new data from DMA
 */
blackfin_dcache_invalidate_range((unsigned long)new_skb->head,
 (unsigned long)new_skb->end);
@@ -1205,7 +1205,7 @@ static void bfin_mac_rx(struct bfin_mac_local *lp)
}
/* reserve 2 bytes for RXDWA padding */
skb_reserve(new_skb, NET_IP_ALIGN);
-   /* Invidate the data cache of skb->data range when it is write back
+   /* Invalidate the data cache of skb->data range when it is write back
 * cache. It will prevent overwritting the new data from DMA
 */
blackfin_dcache_invalidate_range((unsigned long)new_skb->head,
@@ -1599,7 +1599,7 @@ static int bfin_mac_probe(struct platform_device *pdev)
*(__le16 *) (&(ndev->dev_addr[4])) = cpu_to_le16((u16) 
bfin_read_EMAC_ADDRHI());
 
/* probe mac */
-   /*todo: how to proble? which is revision_register */
+   /*todo: how to probe? which is revision_register */
bfin_write_EMAC_ADDRLO(0x12345678);
if (bfin_read_EMAC_ADDRLO() != 0x12345678) {
dev_err(>dev, "Cannot detect Blackfin on-chip ethernet 
MAC controller!\n");
-- 
2.7.3

Re: [PATCH v2 3/5] ARM: sun8i: dt: Add DT bindings documentation for Allwinner sun8i-emac

[LABBE Corentin]

On Thu, Jul 28, 2016 at 08:49:16PM +0200, Maxime Ripard wrote:
> On Thu, Jul 28, 2016 at 03:40:31PM +0200, LABBE Corentin wrote:
> > On Thu, Jul 21, 2016 at 09:55:19AM +0200, Maxime Ripard wrote:
> > > Hi,
> > > 
> > > On Wed, Jul 20, 2016 at 10:03:18AM +0200, LABBE Corentin wrote:
> > > > This patch adds documentation for Device-Tree bindings for the
> > > > Allwinner sun8i-emac driver.
> > > > 
> > > > Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
> > > > ---
> > > >  .../bindings/net/allwinner,sun8i-emac.txt  | 65 
> > > > ++
> > > >  1 file changed, 65 insertions(+)
> > > >  create mode 100644 
> > > > Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt
> > > > 
> > > > diff --git 
> > > > a/Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt 
> > > > b/Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt
> > > > new file mode 100644
> > > > index 000..4bf4e53
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt
> > > > @@ -0,0 +1,65 @@
> > > > +* Allwinner sun8i EMAC ethernet controller
> > > > +
> > > > +Required properties:
> > > > +- compatible: "allwinner,sun8i-a83t-emac", "allwinner,sun8i-h3-emac",
> > > > +   or "allwinner,sun50i-a64-emac"
> > > > +- reg: address and length of the register sets for the device.
> > > > +- reg-names: should be "emac" and "syscon", matching the register sets
> > > 
> > > Blindly mapping a register of some other device on the SoC doesn't
> > > look very reasonable.
> > 
> > As we discuss after this mail on IRC, this register is dedicated to EMAC.
> 
> I don't think we did. It's still right in the middle of some other
> hardware block register space. You actually have a syscon driver to do
> just that, why not use it?
> 

I will try with syscon driver

> > > > +See ethernet.txt in the same directory for generic bindings for 
> > > > ethernet
> > > > +controllers.
> > > > +
> > > > +The device node referenced by "phy" or "phy-handle" should be a child 
> > > > node
> > > > +of this node. See phy.txt for the generic PHY bindings.
> > > > +
> > > > +Optional properties:
> > > > +- phy-supply: phandle to a regulator if the PHY needs one
> > > > +- phy-io-supply: phandle to a regulator if the PHY needs a another one 
> > > > for I/O.
> > > > +This is sometimes found with RGMII PHYs, which use a 
> > > > second
> > > > +regulator for the lower I/O voltage.
> > > > +- allwinner,tx-delay: The setting of the TX clock delay chain
> > > > +- allwinner,rx-delay: The setting of the RX clock delay chain
> > > 
> > > In which unit? What is the default value?
> > 
> > The unit is unknown to me, but I have added a comment for the
> > default and acceptable range value.
> 
> That's unfortunate. We'll see how the DT maintainers feel about that.
> 

I have searched for txdelay in Documentation, and found a few driver that give 
the units (us/ps).
But in that case, the value in ps/us must be found in a table indexed by the 
Xxdelay value.
So the settings seems always a raw number, and for sun8i-emac nothing in user 
manual could help to find what each value is/related to.

So the good value is either found by "try and test" or "copy the value found in 
fex file".

Regards

LABBE Corentin

Re: [PATCH v2 1/5] ethernet: add sun8i-emac driver

[LABBE Corentin]

On Mon, Jul 25, 2016 at 09:54:55PM +0200, Maxime Ripard wrote:
> On Wed, Jul 20, 2016 at 10:03:16AM +0200, LABBE Corentin wrote:
> > This patch add support for sun8i-emac ethernet MAC hardware.
> > It could be found in Allwinner H3/A83T/A64 SoCs.
> > 
> > It supports 10/100/1000 Mbit/s speed with half/full duplex.
> > It can use an internal PHY (MII 10/100) or an external PHY
> > via RGMII/RMII.
> > 
> > Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
> > ---
> >  drivers/net/ethernet/allwinner/Kconfig  |   13 +
> >  drivers/net/ethernet/allwinner/Makefile |1 +
> >  drivers/net/ethernet/allwinner/sun8i-emac.c | 2129 
> > +++
> >  3 files changed, 2143 insertions(+)
> >  create mode 100644 drivers/net/ethernet/allwinner/sun8i-emac.c
> > 
> > diff --git a/drivers/net/ethernet/allwinner/Kconfig 
> > b/drivers/net/ethernet/allwinner/Kconfig
> > index 47da7e7..060569c 100644
> > --- a/drivers/net/ethernet/allwinner/Kconfig
> > +++ b/drivers/net/ethernet/allwinner/Kconfig
> > @@ -33,4 +33,17 @@ config SUN4I_EMAC
> >To compile this driver as a module, choose M here.  The module
> >will be called sun4i-emac.
> >  
> > +config SUN8I_EMAC
> > +   tristate "Allwinner sun8i EMAC support"
> > +   depends on ARCH_SUNXI || COMPILE_TEST
> > +   depends on OF
> > +   select MII
> > +   select PHYLIB
> > +---help---
> > + This driver support the sun8i EMAC ethernet driver present on
> > + H3/A83T/A64 Allwinner SoCs.
> > +
> > +  To compile this driver as a module, choose M here.  The module
> > +  will be called sun8i-emac.
> > +
> >  endif # NET_VENDOR_ALLWINNER
> > diff --git a/drivers/net/ethernet/allwinner/Makefile 
> > b/drivers/net/ethernet/allwinner/Makefile
> > index 03129f7..8bd1693c 100644
> > --- a/drivers/net/ethernet/allwinner/Makefile
> > +++ b/drivers/net/ethernet/allwinner/Makefile
> > @@ -3,3 +3,4 @@
> >  #
> >  
> >  obj-$(CONFIG_SUN4I_EMAC) += sun4i-emac.o
> > +obj-$(CONFIG_SUN8I_EMAC) += sun8i-emac.o
> > diff --git a/drivers/net/ethernet/allwinner/sun8i-emac.c 
> > b/drivers/net/ethernet/allwinner/sun8i-emac.c
> > new file mode 100644
> > index 000..fc0c1dd
> > --- /dev/null
> > +++ b/drivers/net/ethernet/allwinner/sun8i-emac.c
> > @@ -0,0 +1,2129 @@
> > +/*
> > + * sun8i-emac driver
> > + *
> > + * Copyright (C) 2015-2016 Corentin LABBE <clabbe.montj...@gmail.com>
> > + *
> > + * This is the driver for Allwinner Ethernet MAC found in H3/A83T/A64 SoC
> > + *
> > + * TODO:
> > + * - MAC filtering
> > + * - Jumbo frame
> > + * - features rx-all (NETIF_F_RXALL_BIT)
> > + */
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#define SUN8I_EMAC_BASIC_CTL0  0x00
> > +#define SUN8I_EMAC_BASIC_CTL1  0x04
> > +#define SUN8I_EMAC_INT_STA 0x08
> > +#define SUN8I_EMAC_INT_EN  0x0C
> > +#define SUN8I_EMAC_TX_CTL0 0x10
> > +#define SUN8I_EMAC_TX_CTL1 0x14
> > +#define SUN8I_EMAC_TX_FLOW_CTL 0x1C
> > +#define SUN8I_EMAC_RX_CTL0 0x24
> > +#define SUN8I_EMAC_RX_CTL1 0x28
> > +#define SUN8I_EMAC_RX_FRM_FLT  0x38
> > +#define SUN8I_EMAC_MDIO_CMD0x48
> > +#define SUN8I_EMAC_MDIO_DATA   0x4C
> > +#define SUN8I_EMAC_TX_DMA_STA  0xB0
> > +#define SUN8I_EMAC_TX_CUR_DESC 0xB4
> > +#define SUN8I_EMAC_TX_CUR_BUF  0xB8
> > +#define SUN8I_EMAC_RX_DMA_STA  0xC0
> > +
> > +#define MDIO_CMD_MII_BUSY  BIT(0)
> > +#define MDIO_CMD_MII_WRITE BIT(1)
> > +#define MDIO_CMD_MII_PHY_REG_ADDR_MASK GENMASK(8, 4)
> > +#define MDIO_CMD_MII_PHY_REG_ADDR_SHIFT4
> > +#define MDIO_CMD_MII_PHY_ADDR_MASK GENMASK(16, 12)
> > +#define MDIO_CMD_MII_PHY_ADDR_SHIFT12
> > +
> > +#define SUN8I_EMAC_MACADDR_HI  0x50
> > +#define SUN8I_EMAC_MACADDR_LO  0x54
> > +
> > +#define SUN8I_EMAC_RX_DESC_LIST 0x34
> > +#define SUN8I_EMAC_TX_DESC_LIST 0x20
> > +
> > +#define SUN8I_EMAC_RX_DO_CRC BIT(27)
> > +#define SUN8I_EMAC_RX_STRIP_FCS BIT(28)
> > +
> > +#define SUN8I_COULD_BE_USED_BY_DMA BIT(31)
> > +
> &

Re: [PATCH v2 3/5] ARM: sun8i: dt: Add DT bindings documentation for Allwinner sun8i-emac

[LABBE Corentin]

On Thu, Jul 21, 2016 at 09:55:19AM +0200, Maxime Ripard wrote:
> Hi,
> 
> On Wed, Jul 20, 2016 at 10:03:18AM +0200, LABBE Corentin wrote:
> > This patch adds documentation for Device-Tree bindings for the
> > Allwinner sun8i-emac driver.
> > 
> > Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
> > ---
> >  .../bindings/net/allwinner,sun8i-emac.txt  | 65 
> > ++
> >  1 file changed, 65 insertions(+)
> >  create mode 100644 
> > Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt 
> > b/Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt
> > new file mode 100644
> > index 000..4bf4e53
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt
> > @@ -0,0 +1,65 @@
> > +* Allwinner sun8i EMAC ethernet controller
> > +
> > +Required properties:
> > +- compatible: "allwinner,sun8i-a83t-emac", "allwinner,sun8i-h3-emac",
> > +   or "allwinner,sun50i-a64-emac"
> > +- reg: address and length of the register sets for the device.
> > +- reg-names: should be "emac" and "syscon", matching the register sets
> 
> Blindly mapping a register of some other device on the SoC doesn't
> look very reasonable.
> 

As we discuss after this mail on IRC, this register is dedicated to EMAC.

> > +- interrupts: interrupt for the device
> > +- clocks: A phandle to the reference clock for this device
> > +- clock-names: should be "ahb"
> > +- resets: A phandle to the reset control for this device
> > +- reset-names: should be "ahb"
> > +- phy-mode: See ethernet.txt
> > +- phy or phy-handle: See ethernet.txt
> > +- #address-cells: shall be 1
> > +- #size-cells: shall be 0
> > +
> > +"allwinner,sun8i-h3-emac" also requires:
> > +- clocks: an extra phandle to the reference clock for the EPHY
> > +- clock-names: an extra "ephy" entry matching the clocks property
> > +- resets: an extra phandle to the reset control for the EPHY
> > +- resets-names: an extra "ephy" entry matching the resets property
> 
> Shouldn't that be attached to the phy itself?
> 

Ok I will move them.

> > +See ethernet.txt in the same directory for generic bindings for ethernet
> > +controllers.
> > +
> > +The device node referenced by "phy" or "phy-handle" should be a child node
> > +of this node. See phy.txt for the generic PHY bindings.
> > +
> > +Optional properties:
> > +- phy-supply: phandle to a regulator if the PHY needs one
> > +- phy-io-supply: phandle to a regulator if the PHY needs a another one for 
> > I/O.
> > +This is sometimes found with RGMII PHYs, which use a second
> > +regulator for the lower I/O voltage.
> > +- allwinner,tx-delay: The setting of the TX clock delay chain
> > +- allwinner,rx-delay: The setting of the RX clock delay chain
> 
> In which unit? What is the default value?
> 

The unit is unknown to me, but I have added a comment for the default and 
acceptable range value.

> > +
> > +The TX/RX clock delay chain settings are board specific.
> > +
> > +Optional properties for "allwinner,sun8i-h3-emac":
> > +- allwinner,use-internal-phy: Use the H3 SoC's internal E(thernet) PHY
> 
> Can't that be derived from the presence of the phy property?
> 

Yes, I have reworked the "variant" of the driver for easily handling this.

> > +- allwinner,leds-active-low: EPHY LEDs are active low
> 
> That also seems PHY related. Overall, I feel like we really need a phy
> node for the internal phy.
> 

Moved also in the phy node.

> Maxime
> 
> -- 
> Maxime Ripard, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com

Best regards

Thanks

LABBE Corentin

Re: [PATCH v2 3/5] ARM: sun8i: dt: Add DT bindings documentation for Allwinner sun8i-emac

[LABBE Corentin]

On Wed, Jul 20, 2016 at 02:15:33PM -0500, Rob Herring wrote:
> On Wed, Jul 20, 2016 at 10:03:18AM +0200, LABBE Corentin wrote:
> > This patch adds documentation for Device-Tree bindings for the
> > Allwinner sun8i-emac driver.
> > 
> > Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
> > ---
> >  .../bindings/net/allwinner,sun8i-emac.txt  | 65 
> > ++
> >  1 file changed, 65 insertions(+)
> >  create mode 100644 
> > Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt 
> > b/Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt
> > new file mode 100644
> > index 000..4bf4e53
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt
> > @@ -0,0 +1,65 @@
> > +* Allwinner sun8i EMAC ethernet controller
> > +
> > +Required properties:
> > +- compatible: "allwinner,sun8i-a83t-emac", "allwinner,sun8i-h3-emac",
> > +   or "allwinner,sun50i-a64-emac"
> 
> List one per line.
> 
ok

> > +- reg: address and length of the register sets for the device.
> > +- reg-names: should be "emac" and "syscon", matching the register sets
> > +- interrupts: interrupt for the device
> > +- clocks: A phandle to the reference clock for this device
> > +- clock-names: should be "ahb"
> > +- resets: A phandle to the reset control for this device
> > +- reset-names: should be "ahb"
> > +- phy-mode: See ethernet.txt
> > +- phy or phy-handle: See ethernet.txt
> > +- #address-cells: shall be 1
> > +- #size-cells: shall be 0
> > +
> > +"allwinner,sun8i-h3-emac" also requires:
> > +- clocks: an extra phandle to the reference clock for the EPHY
> > +- clock-names: an extra "ephy" entry matching the clocks property
> > +- resets: an extra phandle to the reset control for the EPHY
> > +- resets-names: an extra "ephy" entry matching the resets property
> > +
> > +See ethernet.txt in the same directory for generic bindings for ethernet
> > +controllers.
> > +
> > +The device node referenced by "phy" or "phy-handle" should be a child node
> > +of this node. See phy.txt for the generic PHY bindings.
> > +
> > +Optional properties:
> > +- phy-supply: phandle to a regulator if the PHY needs one
> > +- phy-io-supply: phandle to a regulator if the PHY needs a another one for 
> > I/O.
> > +This is sometimes found with RGMII PHYs, which use a second
> > +regulator for the lower I/O voltage.
> 
> I previously said these should go in the phy node, and you said you 
> would remove them.
> 
> Rob

Sorry, I forgot to remove them.
I have removed them now.

Regards

Thanks

LABBE Corentin

Re: [PATCH v2 1/5] ethernet: add sun8i-emac driver

[LABBE Corentin]

On Wed, Jul 20, 2016 at 11:56:12AM +0200, Arnd Bergmann wrote:
> On Wednesday, July 20, 2016 10:03:16 AM CEST LABBE Corentin wrote:
> > +
> > +   /* Benched on OPIPC with 100M, setting more than 256 does not give 
> > any
> > +* perf boost
> > +*/
> > +   priv->nbdesc_rx = 128;
> > +   priv->nbdesc_tx = 256;
> > +
> > 
> 
> 256 tx descriptors can introduce a significant latency. Can you add
> support for BQL (netdev_sent_queue/netdev_completed_queue) to limit
> the queue size to the minimum?

Done, since setting below 256 give lower performance with iperf.

> 
> I also noticed that your tx_lock() prevents you from concurrently
> running sun8i_emac_complete_xmit() and sun8i_emac_xmit(). Is that
> necessary? I'd think that you can find a way to make them work
> concurrently.
> 
>   Arnd

I will reworked locking and it seems that no locking is necessary.
I have added the following comment about the locking strategy:

/* Locking strategy:
 * RX queue does not need any lock since only sun8i_emac_poll() access it.
 * (All other RX modifiers (ringparam/ndo_stop) disable NAPI and so 
sun8i_emac_poll())
 * TX queue is handled by sun8i_emac_xmit(), sun8i_emac_complete_xmit() and 
sun8i_emac_tx_timeout()
 * (All other RX modifiers (ringparam/ndo_stop) disable NAPI and stop queue)
 *
 * sun8i_emac_xmit() could fire only once (netif_tx_lock)
 * sun8i_emac_complete_xmit() could fire only once (called from NAPI)
 * sun8i_emac_tx_timeout() could fire only once (netif_tx_lock) and couldnt
 * race with sun8i_emac_xmit (due to netif_tx_lock) and with 
sun8i_emac_complete_xmit which disable NAPI.
 *
 * So only sun8i_emac_xmit and sun8i_emac_complete_xmit could fire at the same 
time.
 * But they never could modify the same descriptors:
 * - sun8i_emac_complete_xmit() will modify only descriptors with empty status
 * - sun8i_emac_xmit() will modify only descriptors set to DCLEAN
 * Proper memory barriers ensure that descriptor set to DCLEAN could not be
 * modified latter by sun8i_emac_complete_xmit().
 * */

Does I am right ?

Thanks for your review.

Regards

LABBE Corentin

[PATCH v2 5/5] ARM: dts: sun8i: Enable sun8i-emac on the Orange PI PC

[LABBE Corentin]

The sun8i-emac hardware is present on the Orange PI PC.
It uses the internal PHY.

This patch create the needed emac and phy nodes.

Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
---
 arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts 
b/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
index daf50b9a6..b8340f7 100644
--- a/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
+++ b/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
@@ -165,3 +165,14 @@
/* USB VBUS is always on */
status = "okay";
 };
+
+ {
+   phy = <>;
+   phy-mode = "mii";
+   allwinner,use-internal-phy;
+   allwinner,leds-active-low;
+   status = "okay";
+   phy1: ethernet-phy@1 {
+   reg = <1>;
+   };
+};
-- 
2.7.3

[PATCH v2 4/5] ARM: dts: sun8i-h3: add sun8i-emac ethernet driver

[LABBE Corentin]

The sun8i-emac is an ethernet MAC hardware that support 10/100/1000
speed.

This patch enable the sun8i-emac on the Allwinner H3 SoC Device-tree.
The SoC H3 have an internal PHY, so optionals syscon and ephy are set.

Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
---
 arch/arm/boot/dts/sun8i-h3.dtsi | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
index 4a4926b..f5a95ff 100644
--- a/arch/arm/boot/dts/sun8i-h3.dtsi
+++ b/arch/arm/boot/dts/sun8i-h3.dtsi
@@ -626,6 +626,20 @@
status = "disabled";
};
 
+   emac: ethernet@1c3 {
+   compatible = "allwinner,sun8i-h3-emac";
+   reg = <0x01c3 0x104>, <0x01c00030 0x4>;
+   reg-names = "emac", "syscon";
+   interrupts = ;
+   resets = <_rst 17>, <_rst 66>;
+   reset-names = "ahb", "ephy";
+   clocks = <_gates 17>, <_gates 128>;
+   clock-names = "ahb", "ephy";
+   #address-cells = <1>;
+   #size-cells = <0>;
+   status = "disabled";
+   };
+
gic: interrupt-controller@01c81000 {
compatible = "arm,cortex-a7-gic", "arm,cortex-a15-gic";
reg = <0x01c81000 0x1000>,
-- 
2.7.3

[PATCH v2 3/5] ARM: sun8i: dt: Add DT bindings documentation for Allwinner sun8i-emac

[LABBE Corentin]

This patch adds documentation for Device-Tree bindings for the
Allwinner sun8i-emac driver.

Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
---
 .../bindings/net/allwinner,sun8i-emac.txt  | 65 ++
 1 file changed, 65 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt

diff --git a/Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt 
b/Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt
new file mode 100644
index 000..4bf4e53
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt
@@ -0,0 +1,65 @@
+* Allwinner sun8i EMAC ethernet controller
+
+Required properties:
+- compatible: "allwinner,sun8i-a83t-emac", "allwinner,sun8i-h3-emac",
+   or "allwinner,sun50i-a64-emac"
+- reg: address and length of the register sets for the device.
+- reg-names: should be "emac" and "syscon", matching the register sets
+- interrupts: interrupt for the device
+- clocks: A phandle to the reference clock for this device
+- clock-names: should be "ahb"
+- resets: A phandle to the reset control for this device
+- reset-names: should be "ahb"
+- phy-mode: See ethernet.txt
+- phy or phy-handle: See ethernet.txt
+- #address-cells: shall be 1
+- #size-cells: shall be 0
+
+"allwinner,sun8i-h3-emac" also requires:
+- clocks: an extra phandle to the reference clock for the EPHY
+- clock-names: an extra "ephy" entry matching the clocks property
+- resets: an extra phandle to the reset control for the EPHY
+- resets-names: an extra "ephy" entry matching the resets property
+
+See ethernet.txt in the same directory for generic bindings for ethernet
+controllers.
+
+The device node referenced by "phy" or "phy-handle" should be a child node
+of this node. See phy.txt for the generic PHY bindings.
+
+Optional properties:
+- phy-supply: phandle to a regulator if the PHY needs one
+- phy-io-supply: phandle to a regulator if the PHY needs a another one for I/O.
+This is sometimes found with RGMII PHYs, which use a second
+regulator for the lower I/O voltage.
+- allwinner,tx-delay: The setting of the TX clock delay chain
+- allwinner,rx-delay: The setting of the RX clock delay chain
+
+The TX/RX clock delay chain settings are board specific.
+
+Optional properties for "allwinner,sun8i-h3-emac":
+- allwinner,use-internal-phy: Use the H3 SoC's internal E(thernet) PHY
+- allwinner,leds-active-low: EPHY LEDs are active low
+
+When the internal PHY is requested, the implementation shall configure the
+internal PHY to use the address specified in the child PHY node.
+
+Example:
+
+emac: ethernet@01c0b000 {
+   compatible = "allwinner,sun8i-h3-emac";
+   reg = <0x01c0b000 0x104>, <0x01c00030 0x4>;
+   reg-names = "emac", "syscon";
+   interrupts = ;
+   clocks = <_gates 17>, <_gates 128>;
+   clock-names = "ahb", "ephy";
+   resets = <_rst 17>, <_rst 66>;
+   reset-names = "ahb", "ephy";
+   phy = <>;
+   allwinner,use-internal-phy;
+   allwinner,leds-active-low;
+
+   phy1: ethernet-phy@1 {
+   reg = <1>;
+   };
+};
-- 
2.7.3

[PATCH v2 1/5] ethernet: add sun8i-emac driver

[LABBE Corentin]

This patch add support for sun8i-emac ethernet MAC hardware.
It could be found in Allwinner H3/A83T/A64 SoCs.

It supports 10/100/1000 Mbit/s speed with half/full duplex.
It can use an internal PHY (MII 10/100) or an external PHY
via RGMII/RMII.

Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
---
 drivers/net/ethernet/allwinner/Kconfig  |   13 +
 drivers/net/ethernet/allwinner/Makefile |1 +
 drivers/net/ethernet/allwinner/sun8i-emac.c | 2129 +++
 3 files changed, 2143 insertions(+)
 create mode 100644 drivers/net/ethernet/allwinner/sun8i-emac.c

diff --git a/drivers/net/ethernet/allwinner/Kconfig 
b/drivers/net/ethernet/allwinner/Kconfig
index 47da7e7..060569c 100644
--- a/drivers/net/ethernet/allwinner/Kconfig
+++ b/drivers/net/ethernet/allwinner/Kconfig
@@ -33,4 +33,17 @@ config SUN4I_EMAC
   To compile this driver as a module, choose M here.  The module
   will be called sun4i-emac.
 
+config SUN8I_EMAC
+   tristate "Allwinner sun8i EMAC support"
+   depends on ARCH_SUNXI || COMPILE_TEST
+   depends on OF
+   select MII
+   select PHYLIB
+---help---
+ This driver support the sun8i EMAC ethernet driver present on
+ H3/A83T/A64 Allwinner SoCs.
+
+  To compile this driver as a module, choose M here.  The module
+  will be called sun8i-emac.
+
 endif # NET_VENDOR_ALLWINNER
diff --git a/drivers/net/ethernet/allwinner/Makefile 
b/drivers/net/ethernet/allwinner/Makefile
index 03129f7..8bd1693c 100644
--- a/drivers/net/ethernet/allwinner/Makefile
+++ b/drivers/net/ethernet/allwinner/Makefile
@@ -3,3 +3,4 @@
 #
 
 obj-$(CONFIG_SUN4I_EMAC) += sun4i-emac.o
+obj-$(CONFIG_SUN8I_EMAC) += sun8i-emac.o
diff --git a/drivers/net/ethernet/allwinner/sun8i-emac.c 
b/drivers/net/ethernet/allwinner/sun8i-emac.c
new file mode 100644
index 000..fc0c1dd
--- /dev/null
+++ b/drivers/net/ethernet/allwinner/sun8i-emac.c
@@ -0,0 +1,2129 @@
+/*
+ * sun8i-emac driver
+ *
+ * Copyright (C) 2015-2016 Corentin LABBE <clabbe.montj...@gmail.com>
+ *
+ * This is the driver for Allwinner Ethernet MAC found in H3/A83T/A64 SoC
+ *
+ * TODO:
+ * - MAC filtering
+ * - Jumbo frame
+ * - features rx-all (NETIF_F_RXALL_BIT)
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define SUN8I_EMAC_BASIC_CTL0  0x00
+#define SUN8I_EMAC_BASIC_CTL1  0x04
+#define SUN8I_EMAC_INT_STA 0x08
+#define SUN8I_EMAC_INT_EN  0x0C
+#define SUN8I_EMAC_TX_CTL0 0x10
+#define SUN8I_EMAC_TX_CTL1 0x14
+#define SUN8I_EMAC_TX_FLOW_CTL 0x1C
+#define SUN8I_EMAC_RX_CTL0 0x24
+#define SUN8I_EMAC_RX_CTL1 0x28
+#define SUN8I_EMAC_RX_FRM_FLT  0x38
+#define SUN8I_EMAC_MDIO_CMD0x48
+#define SUN8I_EMAC_MDIO_DATA   0x4C
+#define SUN8I_EMAC_TX_DMA_STA  0xB0
+#define SUN8I_EMAC_TX_CUR_DESC 0xB4
+#define SUN8I_EMAC_TX_CUR_BUF  0xB8
+#define SUN8I_EMAC_RX_DMA_STA  0xC0
+
+#define MDIO_CMD_MII_BUSY  BIT(0)
+#define MDIO_CMD_MII_WRITE BIT(1)
+#define MDIO_CMD_MII_PHY_REG_ADDR_MASK GENMASK(8, 4)
+#define MDIO_CMD_MII_PHY_REG_ADDR_SHIFT4
+#define MDIO_CMD_MII_PHY_ADDR_MASK GENMASK(16, 12)
+#define MDIO_CMD_MII_PHY_ADDR_SHIFT12
+
+#define SUN8I_EMAC_MACADDR_HI  0x50
+#define SUN8I_EMAC_MACADDR_LO  0x54
+
+#define SUN8I_EMAC_RX_DESC_LIST 0x34
+#define SUN8I_EMAC_TX_DESC_LIST 0x20
+
+#define SUN8I_EMAC_RX_DO_CRC BIT(27)
+#define SUN8I_EMAC_RX_STRIP_FCS BIT(28)
+
+#define SUN8I_COULD_BE_USED_BY_DMA BIT(31)
+
+/* Used in RX_CTL1*/
+#define RX_DMA_EN  BIT(30)
+#define RX_DMA_START   BIT(31)
+/* Used in TX_CTL1*/
+#define TX_DMA_EN  BIT(30)
+#define TX_DMA_START   BIT(31)
+
+/* Used in RX_CTL0 */
+#define RX_RECEIVER_EN BIT(31)
+/* Used in TX_CTL0 */
+#define TX_TRANSMITTER_EN  BIT(31)
+
+/* Basic CTL0 */
+#define BCTL0_FD BIT(0)
+#define BCTL0_SPEED_10 2
+#define BCTL0_SPEED_1003
+#define BCTL0_SPEED_MASK   GENMASK(3, 2)
+#define BCTL0_SPEED_SHIFT  2
+
+#define FLOW_RX 1
+#define FLOW_TX 2
+
+#define RX_INT  BIT(8)
+#define TX_INT  BIT(0)
+
+/* Bits used in frame RX status */
+#define DSC_RX_FIRST   BIT(9)
+#define DSC_RX_LASTBIT(8)
+
+/* Bits used in frame TX ctl */
+#define SUN8I_EMAC_MAGIC_TX_BITBIT(24)
+#define SUN8I_EMAC_TX_DO_CRC   (BIT(27) | BIT(28))
+#define DSC_TX_FIRST   BIT(29)
+#define DSC_TX_LASTBIT(30)
+#define SUN8I_EMAC_WANT_INTBIT(31)
+
+enum emac_variant {
+   NONE_EMAC,/* for be sure that variant is non-0 if set */
+   A83T_EMAC,
+   H3_EMAC,
+   A64_EMAC,
+};
+
+static const char const estats_str[][ETH_GSTRING_LEN] = {
+   /* errors */
+   "rx_payload_error",
+   "rx_CRC_error",
+   "rx_phy_error",
+   "rx_length

[PATCH v2 2/5] MAINTAINERS: Add myself as maintainers of sun8i-emac

[LABBE Corentin]

Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
---
 MAINTAINERS | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 1d74837..daefb19 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -581,6 +581,12 @@ S: Maintained
 F: Documentation/i2c/busses/i2c-ali1563
 F: drivers/i2c/busses/i2c-ali1563.c
 
+ALLWINNER SUN8I-EMAC ETHERNET DRIVER
+M: Corentin Labbe <clabbe.montj...@gmail.com>
+L: netdev@vger.kernel.org
+S: Maintained
+F: drivers/net/ethernet/allwinner/sun8i-emac.c
+
 ALLWINNER SECURITY SYSTEM
 M: Corentin Labbe <clabbe.montj...@gmail.com>
 L: linux-cry...@vger.kernel.org
-- 
2.7.3

[PATCH v2 0/5] net-next: ethernet: add sun8i-emac driver

[LABBE Corentin]

Hello

This patch series add the driver for sun8i-emac which handle the Ethernet MAC
present on Allwinner H3/A83T/A64 SoCs.

It supports 10/100/1000 Mbit/s speed with half/full duplex.
It can use an internal PHY (MII 10/100) or an external PHY
via RGMII/RMII.

This patch series enable the driver only for the H3 SoC since A83T and A64
doesn't have the necessary clocks present in mainline.

This patch series enable the driver only for the OrangePiPC board since other
board with H3 use external PHY which need optional regulators that will be
supported later.

The driver have been tested on the following boards:
- H3 Orange PI PC, Orange PI Plus, BananaPI-M2+
- A64 Pine64
- A83T BananaPI-M3

I would like to thanks Chen-Yu Tsai for his help on developing this driver.

Regards

Changes since v1
- Implement NAPI
- Sorted and reworded all define
- Reworked ethtools stats strings
- Removed all unneeded __packked and __aligned
- Added tuning of RX/TX ring size via ethtool
- Corrected use of sk/skb naming
- Added some wmb when needed
- Moved irq claim/free to emac_open/close
- Lots of code refactoring

LABBE Corentin (5):
  ethernet: add sun8i-emac driver
  MAINTAINERS: Add myself as maintainers of sun8i-emac
  ARM: sun8i: dt: Add DT bindings documentation for Allwinner sun8i-emac
  ARM: dts: sun8i-h3: add sun8i-emac ethernet driver
  ARM: dts: sun8i: Enable sun8i-emac on the Orange PI PC

 .../bindings/net/allwinner,sun8i-emac.txt  |   65 +
 MAINTAINERS|6 +
 arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts |   11 +
 arch/arm/boot/dts/sun8i-h3.dtsi|   14 +
 drivers/net/ethernet/allwinner/Kconfig |   13 +
 drivers/net/ethernet/allwinner/Makefile|1 +
 drivers/net/ethernet/allwinner/sun8i-emac.c| 2127 
 7 files changed, 2237 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt
 create mode 100644 drivers/net/ethernet/allwinner/sun8i-emac.c

-- 
2.7.3

Re: [PATCH 1/5] ethernet: add sun8i-emac driver

[LABBE Corentin]

Hello

I agree to all your comments, but for some I have additionnal questions

On Mon, Jun 06, 2016 at 11:25:15AM -0700, Florian Fainelli wrote:
> On 06/03/2016 02:56 AM, LABBE Corentin wrote:
> 
> [snip]
> 
> > +
> > +/* The datasheet said that each descriptor can transfers up to 4096bytes
> > + * But latter, a register documentation reduce that value to 2048
> > + * Anyway using 2048 cause strange behaviours and even BSP driver use 2047
> > + */
> > +#define DESC_BUF_MAX 2044
> > +#if (DESC_BUF_MAX < (ETH_FRAME_LEN + 4))
> > +#error "DESC_BUF_MAX must be set at minimum to ETH_FRAME_LEN + 4"
> > +#endif
> 
> You can probably drop that, it would not make much sense to enable
> fragments and a buffer size smaller than ETH_FRAME_LEN + ETH_FCS_LEN anyway.
> 

I has added this test for preventing someone who want to "optimize" 
DESC_BUF_MAX to doing mistake.
But I agree that it is of low use.

> > +/* Return the number of contiguous free descriptors
> > + * starting from tx_slot
> > + */
> > +static int rb_tx_numfreedesc(struct net_device *ndev)
> > +{
> > +   struct sun8i_emac_priv *priv = netdev_priv(ndev);
> > +
> > +   if (priv->tx_slot < priv->tx_dirty)
> > +   return priv->tx_dirty - priv->tx_slot;
> 
> Does this work with if tx_dirty wraps around?
> 

The tx_dirty cannot wrap since I always keep an empty slot. (tx_slot cannot go 
equal or after tx_dirty)

> > +/* Grab a frame into a skb from descriptor number i */
> > +static int sun8i_emac_rx_from_ddesc(struct net_device *ndev, int i)
> > +{
> > +   struct sk_buff *skb;
> > +   struct sun8i_emac_priv *priv = netdev_priv(ndev);
> > +   struct dma_desc *ddesc = priv->dd_rx + i;
> > +   int frame_len;
> > +   int crc_checked = 0;
> > +
> > +   if (ndev->features & NETIF_F_RXCSUM)
> > +   crc_checked = 1;
> 
> Assuming CRC here refers to the Ethernet frame's FCS, then this is
> absolutely not how NETIF_F_RXCSUM works. NETIF_F_RXCSUM is about your
> Ethernet adapter supporting L3/L4 checksum offloads, while the Ethernet
> FCS is pretty much mandatory for the frame to be properly received in
> the first place. Can you clarify which way it is?
> 

No CRC here is RXCSUM. I understand the misnaming.
I will rename the variable to rxcsum_done.

> > +
> > +   priv->ndev->stats.rx_packets++;
> > +   priv->ndev->stats.rx_bytes += frame_len;
> > +   priv->rx_sk[i] = NULL;
> > +
> > +   /* this frame is not the last */
> > +   if ((ddesc->status & BIT(8)) == 0) {
> > +   dev_warn(priv->dev, "Multi frame not implemented currlen=%d\n",
> > +frame_len);
> > +   }
> > +
> > +   sun8i_emac_rx_sk(ndev, i);
> > +
> > +   netif_rx(skb);
> 
> netif_receive_skb() at the very least, or if you implement NAPI, like
> you shoud napi_gro_receive().
> 

netif_receive_skb documentation say
"This function may only be called from softirq context and interrupts should be 
enabled."
but the calling functions is in hardirq context.

> > +   return 0;
> > +}
> > +
> > +/* Cycle over RX DMA descriptors for finding frame to receive
> > + */
> > +static int sun8i_emac_receive_all(struct net_device *ndev)
> > +{
> > +   struct sun8i_emac_priv *priv = netdev_priv(ndev);
> > +   struct dma_desc *ddesc;
> > +
> > +   ddesc = priv->dd_rx + priv->rx_dirty;
> > +   while (!(ddesc->status & BIT(31))) {
> > +   sun8i_emac_rx_from_ddesc(ndev, priv->rx_dirty);
> > +   rb_inc(>rx_dirty, nbdesc_rx);
> > +   ddesc = priv->dd_rx + priv->rx_dirty;
> > +   };
> 
> So, what if we ping flood your device here, is not there a remote chance
> that we keep the RX interrupt so busy we can't break out of this loop,
> and we are executing from hard IRQ context, that's bad.
> 

I have added a start variable for preventing to do more than a full loop.

> > +
> > +   return 0;
> > +}
> > +
> > +/* iterate over dma_desc for finding completed xmit.
> > + * Called from interrupt context, so no need to spinlock tx
> 
> Humm, well it depends if what you are doing here may race with
> ndo_start_xmit(), and usually it does.
> 

I believe that how it is designed it cannot race each over (access the same 
descriptor slot) since I keep a free slot between each other.

> Also, you should consider completing TX packets in NAPI context (soft
> IRQ) instead of hard IRQs like here.
> 

I wanted to finish this driver the "old" way (with hard IRQ) an

Re: [linux-sunxi] [PATCH 1/5] ethernet: add sun8i-emac driver

[LABBE Corentin]

On Sun, Jun 05, 2016 at 11:32:11PM +0100, André Przywara wrote:
> On 03/06/16 10:56, LABBE Corentin wrote:
> 
> Hi,
> 
> first: thanks for posting this and the time and work that you spent on
> it. With the respective DT nodes this works for me on the Pine64 and
> turns this board eventually into something useful.
> 
> Some comments below:
> 
> > This patch add support for sun8i-emac ethernet MAC hardware.
> > It could be found in Allwinner H3/A83T/A64 SoCs.
> > 
> > It supports 10/100/1000 Mbit/s speed with half/full duplex.
> > It can use an internal PHY (MII 10/100) or an external PHY
> > via RGMII/RMII.
> > 
> > Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
> > ---
> >  drivers/net/ethernet/allwinner/Kconfig  |   13 +
> >  drivers/net/ethernet/allwinner/Makefile |1 +
> >  drivers/net/ethernet/allwinner/sun8i-emac.c | 1943 
> > +++
> >  3 files changed, 1957 insertions(+)
> >  create mode 100644 drivers/net/ethernet/allwinner/sun8i-emac.c
> > 
> > diff --git a/drivers/net/ethernet/allwinner/Kconfig 
> > b/drivers/net/ethernet/allwinner/Kconfig
> > index 47da7e7..226499d 100644
> > --- a/drivers/net/ethernet/allwinner/Kconfig
> > +++ b/drivers/net/ethernet/allwinner/Kconfig
> > @@ -33,4 +33,17 @@ config SUN4I_EMAC
> >To compile this driver as a module, choose M here.  The module
> >will be called sun4i-emac.
> >  
> > +config SUN8I_EMAC
> > +tristate "Allwinner sun8i EMAC support"
> 
> nit: w/s error
> 
ok

> > +
> > +#define SUN8I_EMAC_BASIC_CTL0 0x00
> > +#define SUN8I_EMAC_BASIC_CTL1 0x04
> > +
> > +#define SUN8I_EMAC_MDIO_CMD 0x48
> > +#define SUN8I_EMAC_MDIO_DATA 0x4C
> 
> Can you align all these register offsets with tabs, so that the actual
> offset values are below each other?
> Also ordering them by address seems useful to me.
> 

ok

> > +/* Structure of DMA descriptor used by the hardware  */
> > +struct dma_desc {
> > +   u32 status; /* status of the descriptor */
> > +   u32 st; /* Information on the frame */
> > +   u32 buf_addr; /* physical address of the frame data */
> > +   u32 next; /* physical address of next dma_desc */
> > +} __packed __aligned(4);
> 
> I don't think we need this alignment attribute here:
> 1) The structure will be 4-byte aligned anyway, because the first member
> is naturally 4 bytes aligned.
> 2) The alignment requirement is on the physical DMA side. I don't see
> how the compiler should be able to guarantee this. So technically you
> have to tell the DMA allocation code about your alignment requirement.
> But due to 1), I think this is a moot discussion.
> 

ok, I have removed all __aligned

> 
> 
> 
> > +
> > +
> > +   priv->rx_sk = kcalloc(nbdesc_rx, sizeof(struct sk_buff *), GFP_KERNEL);
> > +   if (!priv->rx_sk) {
> > +   err = -ENOMEM;
> > +   goto rx_sk_error;
> > +   }
> > +   priv->tx_sk = kcalloc(nbdesc_tx, sizeof(struct sk_buff *), GFP_KERNEL);
> > +   if (!priv->tx_sk) {
> > +   err = -ENOMEM;
> > +   goto tx_sk_error;
> > +   }
> > +   priv->tx_map = kcalloc(nbdesc_tx, sizeof(int), GFP_KERNEL);
> > +   if (!priv->tx_map) {
> > +   err = -ENOMEM;
> > +   goto tx_map_error;
> > +   }
> > +
> > +   priv->dd_rx = dma_alloc_coherent(priv->dev,
> > +   nbdesc_rx * sizeof(struct dma_desc),
> > +   >dd_rx_phy,
> > +   GFP_KERNEL);
> 
> You need to be sure here to allocate 32-bit DMA memory only, since the
> hardware holds only 32 bits worth of addresses. If I am not mistaken,
> the following snippet (preferrably in the probe function below) should
> take care of this:
> 
>   if (dma_set_mask_and_coherent(>dev, DMA_BIT_MASK(32))) {
>   dev_err(>dev, "No suitable DMA available\n");
>   return -ENOMEM;
>   }
> 
> This isn't an issue for the SoCs we know of (they have a 32-bit bus
> anyway), but future SoCs could support more memory (the A80 does
> already!), so you have to allocate it from the low 4GB. This is a
> limitation of that particular _device_ (and not the platform), since it
> does the DMA itself and this engine is limited to 32-bit physical addresses.
> 

ok

> > +   if (!priv->dd_rx) {
> > +   dev_err(priv->dev, "ERROR: cannot DMA RX");
> > +   err = -ENOMEM;
> > +   goto dma_rx_error;
> > +   }
> 
&g

[PATCH 2/5] MAINTAINERS: Add myself as maintainers of sun8i-emac

[LABBE Corentin]

Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
---
 MAINTAINERS | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index ed42cb6..d8f5c14 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -581,6 +581,12 @@ S: Maintained
 F: Documentation/i2c/busses/i2c-ali1563
 F: drivers/i2c/busses/i2c-ali1563.c
 
+ALLWINNER SUN8I-EMAC ETHERNET DRIVER
+M: Corentin Labbe <clabbe.montj...@gmail.com>
+L: netdev@vger.kernel.org
+S: Maintained
+F: drivers/net/ethernet/allwinner/sun8i-emac.c
+
 ALLWINNER SECURITY SYSTEM
 M: Corentin Labbe <clabbe.montj...@gmail.com>
 L: linux-cry...@vger.kernel.org
-- 
2.7.3

[PATCH 0/5] net-next: ethernet: add sun8i-emac driver

[LABBE Corentin]

Hello

This patch series add the driver for sun8i-emac which handle the Ethernet MAC
present on Allwinner H3/A83T/A64 SoCs.

It supports 10/100/1000 Mbit/s speed with half/full duplex.
It can use an internal PHY (MII 10/100) or an external PHY
via RGMII/RMII.

This patch series enable the driver only for the H3 SoC since A83T and A64
doesn't have the necessary clocks present in mainline.

This patch series enable the driver only for the OrangePiPC board since other
board with H3 use external PHY which need optional regulators that will be
supported later.

The driver have been tested on the following boards:
- H3 Orange PI PC, Orange PI Plus, BananaPI-M2+
- A64 Pine64
- A83T BananaPI-M3

I would like to thanks Chen-Yu Tsai for his help on developing this driver.

Regards

LABBE Corentin (5):
  ethernet: add sun8i-emac driver
  MAINTAINERS: Add myself as maintainers of sun8i-emac
  ARM: sun8i: dt: Add DT bindings documentation for Allwinner sun8i-emac
  ARM: dts: sun8i-h3: add sun8i-emac ethernet driver
  ARM: dts: sun8i: Enable sun8i-emac on the Orange PI PC

 .../bindings/net/allwinner,sun8i-emac.txt  |   64 +
 MAINTAINERS|6 +
 arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts |   11 +
 arch/arm/boot/dts/sun8i-h3.dtsi|   14 +
 drivers/net/ethernet/allwinner/Kconfig |   13 +
 drivers/net/ethernet/allwinner/Makefile|1 +
 drivers/net/ethernet/allwinner/sun8i-emac.c| 1943 
 7 files changed, 2052 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt
 create mode 100644 drivers/net/ethernet/allwinner/sun8i-emac.c

-- 
2.7.3

[PATCH 1/5] ethernet: add sun8i-emac driver

[LABBE Corentin]

This patch add support for sun8i-emac ethernet MAC hardware.
It could be found in Allwinner H3/A83T/A64 SoCs.

It supports 10/100/1000 Mbit/s speed with half/full duplex.
It can use an internal PHY (MII 10/100) or an external PHY
via RGMII/RMII.

Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
---
 drivers/net/ethernet/allwinner/Kconfig  |   13 +
 drivers/net/ethernet/allwinner/Makefile |1 +
 drivers/net/ethernet/allwinner/sun8i-emac.c | 1943 +++
 3 files changed, 1957 insertions(+)
 create mode 100644 drivers/net/ethernet/allwinner/sun8i-emac.c

diff --git a/drivers/net/ethernet/allwinner/Kconfig 
b/drivers/net/ethernet/allwinner/Kconfig
index 47da7e7..226499d 100644
--- a/drivers/net/ethernet/allwinner/Kconfig
+++ b/drivers/net/ethernet/allwinner/Kconfig
@@ -33,4 +33,17 @@ config SUN4I_EMAC
   To compile this driver as a module, choose M here.  The module
   will be called sun4i-emac.
 
+config SUN8I_EMAC
+tristate "Allwinner sun8i EMAC support"
+   depends on ARCH_SUNXI || COMPILE_TEST
+   depends on OF
+   select MII
+   select PHYLIB
+---help---
+ This driver support the sun8i EMAC ethernet driver present on
+ H3/A83T/A64 Allwinner SoCs.
+
+  To compile this driver as a module, choose M here.  The module
+  will be called sun8i-emac.
+
 endif # NET_VENDOR_ALLWINNER
diff --git a/drivers/net/ethernet/allwinner/Makefile 
b/drivers/net/ethernet/allwinner/Makefile
index 03129f7..8bd1693c 100644
--- a/drivers/net/ethernet/allwinner/Makefile
+++ b/drivers/net/ethernet/allwinner/Makefile
@@ -3,3 +3,4 @@
 #
 
 obj-$(CONFIG_SUN4I_EMAC) += sun4i-emac.o
+obj-$(CONFIG_SUN8I_EMAC) += sun8i-emac.o
diff --git a/drivers/net/ethernet/allwinner/sun8i-emac.c 
b/drivers/net/ethernet/allwinner/sun8i-emac.c
new file mode 100644
index 000..179aa61
--- /dev/null
+++ b/drivers/net/ethernet/allwinner/sun8i-emac.c
@@ -0,0 +1,1943 @@
+/*
+ * sun8i-emac driver
+ *
+ * Copyright (C) 2015-2016 Corentin LABBE <clabbe.montj...@gmail.com>
+ *
+ * This is the driver for Allwinner Ethernet MAC found in H3/A83T/A64 SoC
+ *
+ * TODO:
+ * - NAPI
+ * - MAC filtering
+ * - Jumbo frame
+ * - features rx-all (NETIF_F_RXALL_BIT)
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define SUN8I_EMAC_BASIC_CTL0 0x00
+#define SUN8I_EMAC_BASIC_CTL1 0x04
+
+#define SUN8I_EMAC_MDIO_CMD 0x48
+#define SUN8I_EMAC_MDIO_DATA 0x4C
+
+#define SUN8I_EMAC_RX_CTL0 0x24
+#define SUN8I_EMAC_RX_CTL1 0x28
+
+#define SUN8I_EMAC_TX_CTL0 0x10
+#define SUN8I_EMAC_TX_CTL1 0x14
+
+#define SUN8I_EMAC_TX_FLOW_CTL 0x1C
+
+#define SUN8I_EMAC_RX_FRM_FLT 0x38
+
+#define SUN8I_EMAC_INT_STA 0x08
+#define SUN8I_EMAC_INT_EN 0x0C
+#define SUN8I_EMAC_RGMII_STA 0xD0
+
+#define SUN8I_EMAC_TX_DMA_STA 0xB0
+#define SUN8I_EMAC_TX_CUR_DESC 0xB4
+#define SUN8I_EMAC_TX_CUR_BUF 0xB8
+#define SUN8I_EMAC_RX_DMA_STA 0xC0
+
+#define MDIO_CMD_MII_BUSY  BIT(0)
+#define MDIO_CMD_MII_WRITE BIT(1)
+#define MDIO_CMD_MII_PHY_REG_ADDR_MASK GENMASK(8, 4)
+#define MDIO_CMD_MII_PHY_REG_ADDR_SHIFT4
+#define MDIO_CMD_MII_PHY_ADDR_MASK GENMASK(16, 12)
+#define MDIO_CMD_MII_PHY_ADDR_SHIFT12
+
+#define SUN8I_EMAC_MACADDR_HI  0x50
+#define SUN8I_EMAC_MACADDR_LO  0x54
+
+#define SUN8I_EMAC_RX_DESC_LIST 0x34
+#define SUN8I_EMAC_TX_DESC_LIST 0x20
+
+#define SUN8I_EMAC_TX_DO_CRC (BIT(27) | BIT(28))
+#define SUN8I_EMAC_RX_DO_CRC BIT(27)
+#define SUN8I_EMAC_RX_STRIP_FCS BIT(28)
+
+#define SUN8I_COULD_BE_USED_BY_DMA BIT(31)
+
+#define FLOW_RX 1
+#define FLOW_TX 2
+
+/* describe how data from skb are DMA mapped */
+#define MAP_SINGLE 1
+#define MAP_PAGE 2
+
+enum emac_variant {
+   A83T_EMAC,
+   H3_EMAC,
+   A64_EMAC,
+};
+
+struct ethtool_str {
+   char name[ETH_GSTRING_LEN];
+};
+
+static const struct ethtool_str estats_str[] = {
+   /* errors */
+   { "rx_payload_error" },
+   { "rx_CRC_error" },
+   { "rx_phy_error" },
+   { "rx_length_error" },
+   { "rx_col_error" },
+   { "rx_header_error" },
+   { "rx_overflow_error" },
+   { "rx_saf_error" },
+   { "rx_daf_error" },
+   { "rx_buf_error" },
+   /* misc infos */
+   { "tx_stop_queue" },
+   { "rx_dma_ua" },
+   { "rx_dma_stop" },
+   { "tx_dma_ua" },
+   { "tx_dma_stop" },
+   { "rx_hw_csum" },
+   { "tx_hw_csum" },
+   /* interrupts */
+   { "rx_early_int" },
+   { "tx_early_int" },
+   { "tx_underflow_int" },
+   /* debug */
+   { "tx_used_desc" },
+};
+
+struct sun8i_emac_stats {
+   u64 r

[PATCH 3/5] ARM: sun8i: dt: Add DT bindings documentation for Allwinner sun8i-emac

[LABBE Corentin]

This patch adds documentation for Device-Tree bindings for the
Allwinner sun8i-emac driver.

Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
---
 .../bindings/net/allwinner,sun8i-emac.txt  | 64 ++
 1 file changed, 64 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt

diff --git a/Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt 
b/Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt
new file mode 100644
index 000..cf71a71
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/allwinner,sun8i-emac.txt
@@ -0,0 +1,64 @@
+* Allwinner sun8i EMAC ethernet controller
+
+Required properties:
+- compatible: "allwinner,sun8i-a83t-emac", "allwinner,sun8i-h3-emac",
+   or "allwinner,sun50i-a64-emac"
+- reg: address and length of the register sets for the device.
+- reg-names: should be "emac" and "syscon", matching the register sets
+- interrupts: interrupt for the device
+- clocks: A phandle to the reference clock for this device
+- clock-names: should be "ahb"
+- resets: A phandle to the reset control for this device
+- reset-names: should be "ahb"
+- phy-mode: See ethernet.txt
+- phy or phy-handle: See ethernet.txt
+- #address-cells: shall be 1
+- #size-cells: shall be 0
+
+"allwinner,sun8i-h3-emac" also requires:
+- clocks: an extra phandle to the reference clock for the EPHY
+- clock-names: an extra "ephy" entry matching the clocks property
+- resets: an extra phandle to the reset control for the EPHY
+- resets-names: an extra "ephy" entry matching the resets property
+
+See ethernet.txt in the same directory for generic bindings for ethernet
+controllers.
+
+The device node referenced by "phy" or "phy-handle" should be a child node
+of this node. See phy.txt for the generic PHY bindings.
+
+Optional properties:
+- phy-supply: phandle to a regulator if the PHY needs one
+- phy-io-supply: phandle to a regulator if the PHY needs a another one for I/O.
+This is sometimes found with RGMII PHYs, which use a second
+regulator for the lower I/O voltage.
+- allwinner,tx-delay: The setting of the TX clock delay chain
+- allwinner,rx-delay: The setting of the RX clock delay chain
+
+The TX/RX clock delay chain settings are board specific.
+
+Optional properties for "allwinner,sun8i-h3-emac":
+- allwinner,use-internal-phy: Use the H3 SoC's internal E(thernet) PHY
+- allwinner,leds-active-low: EPHY LEDs are active low
+
+When the internal PHY is requested, the implementation shall configure the
+internal PHY to use the address specified in the child PHY node.
+
+Example:
+
+emac: ethernet@01c0b000 {
+   compatible = "allwinner,sun8i-h3-emac";
+   reg = <0x01c0b000 0x1000>;
+   interrupts = ;
+   clocks = <_gates 17>, <_gates 128>;
+   clock-names = "ahb", "ephy";
+   resets = <_rst 17>, <_rst 66>;
+   reset-names = "ahb", "ephy";
+   phy = <>;
+   allwinner,use-internal-phy;
+   allwinner,leds-active-low;
+
+   phy1: ethernet-phy@1 {
+   reg = <1>;
+   };
+};
-- 
2.7.3

[PATCH 4/5] ARM: dts: sun8i-h3: add sun8i-emac ethernet driver

[LABBE Corentin]

The sun8i-emac is an ethernet MAC hardware that support 10/100/1000
speed.

This patch enable the sun8i-emac on the Allwinner H3 SoC Device-tree.
The SoC H3 have an internal PHY, so optionals syscon and ephy are set.

Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
---
 arch/arm/boot/dts/sun8i-h3.dtsi | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
index 4a4926b..f5a95ff 100644
--- a/arch/arm/boot/dts/sun8i-h3.dtsi
+++ b/arch/arm/boot/dts/sun8i-h3.dtsi
@@ -626,6 +626,20 @@
status = "disabled";
};
 
+   emac: ethernet@1c3 {
+   compatible = "allwinner,sun8i-h3-emac";
+   reg = <0x01c3 0x104>, <0x01c00030 0x4>;
+   reg-names = "emac", "syscon";
+   interrupts = ;
+   resets = <_rst 17>, <_rst 66>;
+   reset-names = "ahb", "ephy";
+   clocks = <_gates 17>, <_gates 128>;
+   clock-names = "ahb", "ephy";
+   #address-cells = <1>;
+   #size-cells = <0>;
+   status = "disabled";
+   };
+
gic: interrupt-controller@01c81000 {
compatible = "arm,cortex-a7-gic", "arm,cortex-a15-gic";
reg = <0x01c81000 0x1000>,
-- 
2.7.3

[PATCH 5/5] ARM: dts: sun8i: Enable sun8i-emac on the Orange PI PC

[LABBE Corentin]

The sun8i-emac hardware is present on the Orange PI PC.
It uses the internal PHY.

This patch create the needed emac and phy nodes.

Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
---
 arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts 
b/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
index daf50b9a6..b8340f7 100644
--- a/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
+++ b/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
@@ -165,3 +165,14 @@
/* USB VBUS is always on */
status = "okay";
 };
+
+ {
+   phy = <>;
+   phy-mode = "mii";
+   allwinner,use-internal-phy;
+   allwinner,leds-active-low;
+   status = "okay";
+   phy1: ethernet-phy@1 {
+   reg = <1>;
+   };
+};
-- 
2.7.3

[PATCH v2] phy: remove documentation of removed members of phy_device structure

[LABBE Corentin]

Commit e5a03bfd873c ("phy: Add an mdio_device structure") removed addr,
bus and dev member of the phy_device structure.
This patch remove the documentation about those members.

Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
---
 include/linux/phy.h | 3 ---
 1 file changed, 3 deletions(-)

Changes since v1
- remove also documentation for dev and bus

diff --git a/include/linux/phy.h b/include/linux/phy.h
index d6f3641..2abd791 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -327,8 +327,6 @@ struct phy_c45_device_ids {
 /* phy_device: An instance of a PHY
  *
  * drv: Pointer to the driver for this PHY instance
- * bus: Pointer to the bus this PHY is on
- * dev: driver model device structure for this PHY
  * phy_id: UID for this device found during discovery
  * c45_ids: 802.3-c45 Device Identifers if is_c45.
  * is_c45:  Set to true if this phy uses clause 45 addressing.
@@ -338,7 +336,6 @@ struct phy_c45_device_ids {
  * suspended: Set to true if this phy has been suspended successfully.
  * state: state of the PHY for management purposes
  * dev_flags: Device-specific flags used by the PHY driver.
- * addr: Bus address of PHY
  * link_timeout: The number of timer firings to wait before the
  * giving up on the current attempt at acquiring a link
  * irq: IRQ number of the PHY's interrupt (-1 if none)
-- 
2.4.10

Re: [PATCH] phy: remove documentation of removed member addr of phy_device structure

[LABBE Corentin]

On Thu, Mar 03, 2016 at 04:14:19PM +0100, Andrew Lunn wrote:
> On Thu, Mar 03, 2016 at 02:12:28PM +0100, LABBE Corentin wrote:
> > Commit e5a03bfd873c ("phy: Add an mdio_device structure") removed
> > the addr member of the phy_device structure.
> > This patch remove the documentation about that member.
> > 
> > Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
> 
> Hi Corentin
> 
> The patch also removed bus and dev which are documented.  Could you
> please expand you patch to remove those as well?
> 
> Thanks
>   Andrew
> 

Yes, I will send a v2 soon.

Regards

LABBE Corentin

[PATCH] phy: remove documentation of removed member addr of phy_device structure

[LABBE Corentin]

Commit e5a03bfd873c ("phy: Add an mdio_device structure") removed
the addr member of the phy_device structure.
This patch remove the documentation about that member.

Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
---
 include/linux/phy.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/linux/phy.h b/include/linux/phy.h
index d6f3641..e14569e 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -338,7 +338,6 @@ struct phy_c45_device_ids {
  * suspended: Set to true if this phy has been suspended successfully.
  * state: state of the PHY for management purposes
  * dev_flags: Device-specific flags used by the PHY driver.
- * addr: Bus address of PHY
  * link_timeout: The number of timer firings to wait before the
  * giving up on the current attempt at acquiring a link
  * irq: IRQ number of the PHY's interrupt (-1 if none)
-- 
2.4.10

[PATCH v2 1/1] cxgb3: Convert simple_strtoul to kstrtox

[LABBE Corentin]

the simple_strtoul function is obsolete. This patch replace it by
kstrtox.

Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
---
 drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c | 14 +++--
 drivers/net/ethernet/chelsio/cxgb3/t3_hw.c  | 28 ++---
 2 files changed, 29 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c 
b/drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c
index 8f7aa53..60908ea 100644
--- a/drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c
@@ -701,15 +701,16 @@ static ssize_t attr_store(struct device *d,
  ssize_t(*set) (struct net_device *, unsigned int),
  unsigned int min_val, unsigned int max_val)
 {
-   char *endp;
ssize_t ret;
unsigned int val;
 
if (!capable(CAP_NET_ADMIN))
return -EPERM;
 
-   val = simple_strtoul(buf, , 0);
-   if (endp == buf || val < min_val || val > max_val)
+   ret = kstrtouint(buf, 0, );
+   if (ret)
+   return ret;
+   if (val < min_val || val > max_val)
return -EINVAL;
 
rtnl_lock();
@@ -829,14 +830,15 @@ static ssize_t tm_attr_store(struct device *d,
struct port_info *pi = netdev_priv(to_net_dev(d));
struct adapter *adap = pi->adapter;
unsigned int val;
-   char *endp;
ssize_t ret;
 
if (!capable(CAP_NET_ADMIN))
return -EPERM;
 
-   val = simple_strtoul(buf, , 0);
-   if (endp == buf || val > 1000)
+   ret = kstrtouint(buf, 0, );
+   if (ret)
+   return ret;
+   if (val > 1000)
return -EINVAL;
 
rtnl_lock();
diff --git a/drivers/net/ethernet/chelsio/cxgb3/t3_hw.c 
b/drivers/net/ethernet/chelsio/cxgb3/t3_hw.c
index a22768c..ee04caa 100644
--- a/drivers/net/ethernet/chelsio/cxgb3/t3_hw.c
+++ b/drivers/net/ethernet/chelsio/cxgb3/t3_hw.c
@@ -709,11 +709,21 @@ static int get_vpd_params(struct adapter *adapter, struct 
vpd_params *p)
return ret;
}
 
-   p->cclk = simple_strtoul(vpd.cclk_data, NULL, 10);
-   p->mclk = simple_strtoul(vpd.mclk_data, NULL, 10);
-   p->uclk = simple_strtoul(vpd.uclk_data, NULL, 10);
-   p->mdc = simple_strtoul(vpd.mdc_data, NULL, 10);
-   p->mem_timing = simple_strtoul(vpd.mt_data, NULL, 10);
+   ret = kstrtouint(vpd.cclk_data, 10, >cclk);
+   if (ret)
+   return ret;
+   ret = kstrtouint(vpd.mclk_data, 10, >mclk);
+   if (ret)
+   return ret;
+   ret = kstrtouint(vpd.uclk_data, 10, >uclk);
+   if (ret)
+   return ret;
+   ret = kstrtouint(vpd.mdc_data, 10, >mdc);
+   if (ret)
+   return ret;
+   ret = kstrtouint(vpd.mt_data, 10, >mem_timing);
+   if (ret)
+   return ret;
memcpy(p->sn, vpd.sn_data, SERNUM_LEN);
 
/* Old eeproms didn't have port information */
@@ -723,8 +733,12 @@ static int get_vpd_params(struct adapter *adapter, struct 
vpd_params *p)
} else {
p->port_type[0] = hex_to_bin(vpd.port0_data[0]);
p->port_type[1] = hex_to_bin(vpd.port1_data[0]);
-   p->xauicfg[0] = simple_strtoul(vpd.xaui0cfg_data, NULL, 16);
-   p->xauicfg[1] = simple_strtoul(vpd.xaui1cfg_data, NULL, 16);
+   ret = kstrtou16(vpd.xaui0cfg_data, 16, >xauicfg[0]);
+   if (ret)
+   return ret;
+   ret = kstrtou16(vpd.xaui1cfg_data, 16, >xauicfg[1]);
+   if (ret)
+   return ret;
}
 
ret = hex2bin(p->eth_base, vpd.na_data, 6);
-- 
2.4.10

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

[PATCH v2 0/1] cxgb3: Convert simple_strtoul to kstrtox

[LABBE Corentin]

Hello

Change since v1
- Always return kstrtox error code

LABBE Corentin (1):
  cxgb3: Convert simple_strtoul to kstrtox

 drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c | 14 +++--
 drivers/net/ethernet/chelsio/cxgb3/t3_hw.c  | 28 ++---
 2 files changed, 29 insertions(+), 13 deletions(-)

-- 
2.4.10

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

[PATCH] cxgb3: Convert simple_strtoul to kstrtox

[LABBE Corentin]

the simple_strtoul function is obsolete. This patch replace it by
kstrtox.

Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
---
 drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c | 10 -
 drivers/net/ethernet/chelsio/cxgb3/t3_hw.c  | 28 ++---
 2 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c 
b/drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c
index 8f7aa53..6637073 100644
--- a/drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c
@@ -701,15 +701,14 @@ static ssize_t attr_store(struct device *d,
  ssize_t(*set) (struct net_device *, unsigned int),
  unsigned int min_val, unsigned int max_val)
 {
-   char *endp;
ssize_t ret;
unsigned int val;
 
if (!capable(CAP_NET_ADMIN))
return -EPERM;
 
-   val = simple_strtoul(buf, , 0);
-   if (endp == buf || val < min_val || val > max_val)
+   ret = kstrtouint(buf, 0, );
+   if (ret || val < min_val || val > max_val)
return -EINVAL;
 
rtnl_lock();
@@ -829,14 +828,13 @@ static ssize_t tm_attr_store(struct device *d,
struct port_info *pi = netdev_priv(to_net_dev(d));
struct adapter *adap = pi->adapter;
unsigned int val;
-   char *endp;
ssize_t ret;
 
if (!capable(CAP_NET_ADMIN))
return -EPERM;
 
-   val = simple_strtoul(buf, , 0);
-   if (endp == buf || val > 1000)
+   ret = kstrtouint(buf, 0, );
+   if (ret || val > 1000)
return -EINVAL;
 
rtnl_lock();
diff --git a/drivers/net/ethernet/chelsio/cxgb3/t3_hw.c 
b/drivers/net/ethernet/chelsio/cxgb3/t3_hw.c
index a22768c..ee04caa 100644
--- a/drivers/net/ethernet/chelsio/cxgb3/t3_hw.c
+++ b/drivers/net/ethernet/chelsio/cxgb3/t3_hw.c
@@ -709,11 +709,21 @@ static int get_vpd_params(struct adapter *adapter, struct 
vpd_params *p)
return ret;
}
 
-   p->cclk = simple_strtoul(vpd.cclk_data, NULL, 10);
-   p->mclk = simple_strtoul(vpd.mclk_data, NULL, 10);
-   p->uclk = simple_strtoul(vpd.uclk_data, NULL, 10);
-   p->mdc = simple_strtoul(vpd.mdc_data, NULL, 10);
-   p->mem_timing = simple_strtoul(vpd.mt_data, NULL, 10);
+   ret = kstrtouint(vpd.cclk_data, 10, >cclk);
+   if (ret)
+   return ret;
+   ret = kstrtouint(vpd.mclk_data, 10, >mclk);
+   if (ret)
+   return ret;
+   ret = kstrtouint(vpd.uclk_data, 10, >uclk);
+   if (ret)
+   return ret;
+   ret = kstrtouint(vpd.mdc_data, 10, >mdc);
+   if (ret)
+   return ret;
+   ret = kstrtouint(vpd.mt_data, 10, >mem_timing);
+   if (ret)
+   return ret;
memcpy(p->sn, vpd.sn_data, SERNUM_LEN);
 
/* Old eeproms didn't have port information */
@@ -723,8 +733,12 @@ static int get_vpd_params(struct adapter *adapter, struct 
vpd_params *p)
} else {
p->port_type[0] = hex_to_bin(vpd.port0_data[0]);
p->port_type[1] = hex_to_bin(vpd.port1_data[0]);
-   p->xauicfg[0] = simple_strtoul(vpd.xaui0cfg_data, NULL, 16);
-   p->xauicfg[1] = simple_strtoul(vpd.xaui1cfg_data, NULL, 16);
+   ret = kstrtou16(vpd.xaui0cfg_data, 16, >xauicfg[0]);
+   if (ret)
+   return ret;
+   ret = kstrtou16(vpd.xaui1cfg_data, 16, >xauicfg[1]);
+   if (ret)
+   return ret;
}
 
ret = hex2bin(p->eth_base, vpd.na_data, 6);
-- 
2.4.10

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

[PATCH v4 1/1] atm: solos-pci: Replace simple_strtol by kstrtoint

[LABBE Corentin]

The simple_strtol function is obsolete.
This patch replace it by kstrtoint.
This will simplify code, since some error case not handled by
simple_strtol are handled by kstrtoint.

Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
---
 drivers/atm/solos-pci.c | 28 +---
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/drivers/atm/solos-pci.c b/drivers/atm/solos-pci.c
index 3d7fb65..0c2b4ba0 100644
--- a/drivers/atm/solos-pci.c
+++ b/drivers/atm/solos-pci.c
@@ -347,8 +347,8 @@ static char *next_string(struct sk_buff *skb)
  */   
 static int process_status(struct solos_card *card, int port, struct sk_buff 
*skb)
 {
-   char *str, *end, *state_str, *snr, *attn;
-   int ver, rate_up, rate_down;
+   char *str, *state_str, *snr, *attn;
+   int ver, rate_up, rate_down, err;
 
if (!card->atmdev[port])
return -ENODEV;
@@ -357,7 +357,11 @@ static int process_status(struct solos_card *card, int 
port, struct sk_buff *skb
if (!str)
return -EIO;
 
-   ver = simple_strtol(str, NULL, 10);
+   err = kstrtoint(str, 10, );
+   if (err) {
+   dev_warn(>dev->dev, "Unexpected status interrupt 
version\n");
+   return err;
+   }
if (ver < 1) {
dev_warn(>dev->dev, "Unexpected status interrupt version 
%d\n",
 ver);
@@ -373,16 +377,16 @@ static int process_status(struct solos_card *card, int 
port, struct sk_buff *skb
return 0;
}
 
-   rate_down = simple_strtol(str, , 10);
-   if (*end)
-   return -EIO;
+   err = kstrtoint(str, 10, _down);
+   if (err)
+   return err;
 
str = next_string(skb);
if (!str)
return -EIO;
-   rate_up = simple_strtol(str, , 10);
-   if (*end)
-   return -EIO;
+   err = kstrtoint(str, 10, _up);
+   if (err)
+   return err;
 
state_str = next_string(skb);
if (!state_str)
@@ -417,7 +421,7 @@ static int process_command(struct solos_card *card, int 
port, struct sk_buff *sk
struct solos_param *prm;
unsigned long flags;
int cmdpid;
-   int found = 0;
+   int found = 0, err;
 
if (skb->len < 7)
return 0;
@@ -428,7 +432,9 @@ static int process_command(struct solos_card *card, int 
port, struct sk_buff *sk
skb->data[6] != '\n')
return 0;
 
-   cmdpid = simple_strtol(>data[1], NULL, 10);
+   err = kstrtoint(>data[1], 10, );
+   if (err)
+   return err;
 
spin_lock_irqsave(>param_queue_lock, flags);
list_for_each_entry(prm, >param_queue, list) {
-- 
2.4.10

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

[PATCH v4 0/1] atm: solos-pci: Replace simple_strtol by kstrtoint

[LABBE Corentin]

Hello

Change since v3
- rework the test logic with ver/err

Change since v2
- Invert a test logic

Change since v1
- Always return error code from kstrtox.

LABBE Corentin (1):
  atm: solos-pci: Replace simple_strtol by kstrtoint

 drivers/atm/solos-pci.c | 28 +---
 1 file changed, 17 insertions(+), 11 deletions(-)

-- 
2.4.10

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

Re: [PATCH v3 1/1] atm: solos-pci: Replace simple_strtol by kstrtoint

[LABBE Corentin]

On Thu, Dec 03, 2015 at 06:26:31AM -0500, Charles (Chas) Williams wrote:
> On Thu, 2015-12-03 at 09:06 +0100, LABBE Corentin wrote:
> > @@ -357,11 +357,11 @@ static int process_status(struct solos_card *card, 
> > int port, struct sk_buff *skb
> > if (!str)
> > return -EIO;
> >  
> > -   ver = simple_strtol(str, NULL, 10);
> > -   if (ver < 1) {
> > +   err = kstrtoint(str, 10, );
> > +   if (err || ver < 1) {
> > dev_warn(>dev->dev, "Unexpected status interrupt version 
> > %d\n",
> >  ver);
> > -   return -EIO;
> > +   return err;
> 
> 
> If ver < 1 then you might return a 0 here.  Always returning -EIO is
> probably just fine.
> 

Hello

I think the best solution is to split the test, since returning error code from 
kstrtoint was asked by David Miller.
if (err)
return err;
if (ver < 1)
return -EIO;
Thanks
Regards
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 1/1] atm: solos-pci: Replace simple_strtol by kstrtoint

[LABBE Corentin]

The simple_strtol function is obsolete.
This patch replace it by kstrtoint.
This will simplify code, since some error case not handled by
simple_strtol are handled by kstrtoint.

Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
---
 drivers/atm/solos-pci.c | 28 +++-
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/drivers/atm/solos-pci.c b/drivers/atm/solos-pci.c
index 3d7fb65..2bec4f1 100644
--- a/drivers/atm/solos-pci.c
+++ b/drivers/atm/solos-pci.c
@@ -347,8 +347,8 @@ static char *next_string(struct sk_buff *skb)
  */   
 static int process_status(struct solos_card *card, int port, struct sk_buff 
*skb)
 {
-   char *str, *end, *state_str, *snr, *attn;
-   int ver, rate_up, rate_down;
+   char *str, *state_str, *snr, *attn;
+   int ver, rate_up, rate_down, err;
 
if (!card->atmdev[port])
return -ENODEV;
@@ -357,11 +357,11 @@ static int process_status(struct solos_card *card, int 
port, struct sk_buff *skb
if (!str)
return -EIO;
 
-   ver = simple_strtol(str, NULL, 10);
-   if (ver < 1) {
+   err = kstrtoint(str, 10, );
+   if (err || ver < 1) {
dev_warn(>dev->dev, "Unexpected status interrupt version 
%d\n",
 ver);
-   return -EIO;
+   return err;
}
 
str = next_string(skb);
@@ -373,16 +373,16 @@ static int process_status(struct solos_card *card, int 
port, struct sk_buff *skb
return 0;
}
 
-   rate_down = simple_strtol(str, , 10);
-   if (*end)
-   return -EIO;
+   err = kstrtoint(str, 10, _down);
+   if (err)
+   return err;
 
str = next_string(skb);
if (!str)
return -EIO;
-   rate_up = simple_strtol(str, , 10);
-   if (*end)
-   return -EIO;
+   err = kstrtoint(str, 10, _up);
+   if (err)
+   return err;
 
state_str = next_string(skb);
if (!state_str)
@@ -417,7 +417,7 @@ static int process_command(struct solos_card *card, int 
port, struct sk_buff *sk
struct solos_param *prm;
unsigned long flags;
int cmdpid;
-   int found = 0;
+   int found = 0, err;
 
if (skb->len < 7)
return 0;
@@ -428,7 +428,9 @@ static int process_command(struct solos_card *card, int 
port, struct sk_buff *sk
skb->data[6] != '\n')
return 0;
 
-   cmdpid = simple_strtol(>data[1], NULL, 10);
+   err = kstrtoint(>data[1], 10, );
+   if (err)
+   return err;
 
spin_lock_irqsave(>param_queue_lock, flags);
list_for_each_entry(prm, >param_queue, list) {
-- 
2.4.10

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

[PATCH v3 0/1] atm: solos-pci: Replace simple_strtol by kstrtoint

[LABBE Corentin]

Hello

Change since v2
- Invert a test logic

Change since v1
- Always return error code from kstrtox.

LABBE Corentin (1):
  atm: solos-pci: Replace simple_strtol by kstrtoint

 drivers/atm/solos-pci.c | 28 +++-
 1 file changed, 15 insertions(+), 13 deletions(-)

-- 
2.4.10

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

Re: [PATCH v2 1/1] atm: solos-pci: Replace simple_strtol by kstrtoint

[LABBE Corentin]

On Wed, Dec 02, 2015 at 05:02:19PM +0300, Sergei Shtylyov wrote:
> Hello.
> 
> On 12/2/2015 3:54 PM, LABBE Corentin wrote:
> 
> > The simple_strtol function is obsolete.
> > This patch replace it by kstrtoint.
> > This will simplify code, since some error case not handled by
> > simple_strtol are handled by kstrtoint.
> >
> > Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
> > ---
> >   drivers/atm/solos-pci.c | 28 +++-
> >   1 file changed, 15 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/atm/solos-pci.c b/drivers/atm/solos-pci.c
> > index 3d7fb65..f944d75 100644
> > --- a/drivers/atm/solos-pci.c
> > +++ b/drivers/atm/solos-pci.c
> > @@ -347,8 +347,8 @@ static char *next_string(struct sk_buff *skb)
> >*/
> >   static int process_status(struct solos_card *card, int port, struct 
> > sk_buff *skb)
> >   {
> > -   char *str, *end, *state_str, *snr, *attn;
> > -   int ver, rate_up, rate_down;
> > +   char *str, *state_str, *snr, *attn;
> > +   int ver, rate_up, rate_down, err;
> >
> > if (!card->atmdev[port])
> > return -ENODEV;
> > @@ -357,11 +357,11 @@ static int process_status(struct solos_card *card, 
> > int port, struct sk_buff *skb
> > if (!str)
> > return -EIO;
> >
> > -   ver = simple_strtol(str, NULL, 10);
> > -   if (ver < 1) {
> > +   err = kstrtoint(str, 10, );
> > +   if (ver < 1 || err) {
> 
> Is 'ver' initialized in case of error? If not, you have to check 'err' 
> first.

Hello

Whatever if ver is initialized, since the conditional is an or, the test will 
always be true with the err value.
Anyway I will send an updated version.

Regards

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

[PATCH v2 0/1] atm: solos-pci: Replace simple_strtol by kstrtoint

[LABBE Corentin]

Hello

Change since v1
- Always return error code from kstrtox.

LABBE Corentin (1):
  atm: solos-pci: Replace simple_strtol by kstrtoint

 drivers/atm/solos-pci.c | 28 +++-
 1 file changed, 15 insertions(+), 13 deletions(-)

-- 
2.4.10

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

[PATCH v2 1/1] atm: solos-pci: Replace simple_strtol by kstrtoint

[LABBE Corentin]

The simple_strtol function is obsolete.
This patch replace it by kstrtoint.
This will simplify code, since some error case not handled by
simple_strtol are handled by kstrtoint.

Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
---
 drivers/atm/solos-pci.c | 28 +++-
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/drivers/atm/solos-pci.c b/drivers/atm/solos-pci.c
index 3d7fb65..f944d75 100644
--- a/drivers/atm/solos-pci.c
+++ b/drivers/atm/solos-pci.c
@@ -347,8 +347,8 @@ static char *next_string(struct sk_buff *skb)
  */   
 static int process_status(struct solos_card *card, int port, struct sk_buff 
*skb)
 {
-   char *str, *end, *state_str, *snr, *attn;
-   int ver, rate_up, rate_down;
+   char *str, *state_str, *snr, *attn;
+   int ver, rate_up, rate_down, err;
 
if (!card->atmdev[port])
return -ENODEV;
@@ -357,11 +357,11 @@ static int process_status(struct solos_card *card, int 
port, struct sk_buff *skb
if (!str)
return -EIO;
 
-   ver = simple_strtol(str, NULL, 10);
-   if (ver < 1) {
+   err = kstrtoint(str, 10, );
+   if (ver < 1 || err) {
dev_warn(>dev->dev, "Unexpected status interrupt version 
%d\n",
 ver);
-   return -EIO;
+   return err;
}
 
str = next_string(skb);
@@ -373,16 +373,16 @@ static int process_status(struct solos_card *card, int 
port, struct sk_buff *skb
return 0;
}
 
-   rate_down = simple_strtol(str, , 10);
-   if (*end)
-   return -EIO;
+   err = kstrtoint(str, 10, _down);
+   if (err)
+   return err;
 
str = next_string(skb);
if (!str)
return -EIO;
-   rate_up = simple_strtol(str, , 10);
-   if (*end)
-   return -EIO;
+   err = kstrtoint(str, 10, _up);
+   if (err)
+   return err;
 
state_str = next_string(skb);
if (!state_str)
@@ -417,7 +417,7 @@ static int process_command(struct solos_card *card, int 
port, struct sk_buff *sk
struct solos_param *prm;
unsigned long flags;
int cmdpid;
-   int found = 0;
+   int found = 0, err;
 
if (skb->len < 7)
return 0;
@@ -428,7 +428,9 @@ static int process_command(struct solos_card *card, int 
port, struct sk_buff *sk
skb->data[6] != '\n')
return 0;
 
-   cmdpid = simple_strtol(>data[1], NULL, 10);
+   err = kstrtoint(>data[1], 10, );
+   if (err)
+   return err;
 
spin_lock_irqsave(>param_queue_lock, flags);
list_for_each_entry(prm, >param_queue, list) {
-- 
2.4.10

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

[PATCH] atm: solos-pci: Replace simple_strtol by kstrtoint

[LABBE Corentin]

The simple_strtol function is obsolete.
This patch replace it by kstrtoint.
This will simplify code, since some error case not handled by
simple_strtol are handled by kstrtoint.

Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
---
 drivers/atm/solos-pci.c | 22 --
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/atm/solos-pci.c b/drivers/atm/solos-pci.c
index 3d7fb65..17a06bd 100644
--- a/drivers/atm/solos-pci.c
+++ b/drivers/atm/solos-pci.c
@@ -347,8 +347,8 @@ static char *next_string(struct sk_buff *skb)
  */   
 static int process_status(struct solos_card *card, int port, struct sk_buff 
*skb)
 {
-   char *str, *end, *state_str, *snr, *attn;
-   int ver, rate_up, rate_down;
+   char *str, *state_str, *snr, *attn;
+   int ver, rate_up, rate_down, err;
 
if (!card->atmdev[port])
return -ENODEV;
@@ -357,8 +357,8 @@ static int process_status(struct solos_card *card, int 
port, struct sk_buff *skb
if (!str)
return -EIO;
 
-   ver = simple_strtol(str, NULL, 10);
-   if (ver < 1) {
+   err = kstrtoint(str, 10, );
+   if (ver < 1 || err) {
dev_warn(>dev->dev, "Unexpected status interrupt version 
%d\n",
 ver);
return -EIO;
@@ -373,15 +373,15 @@ static int process_status(struct solos_card *card, int 
port, struct sk_buff *skb
return 0;
}
 
-   rate_down = simple_strtol(str, , 10);
-   if (*end)
+   err = kstrtoint(str, 10, _down);
+   if (err)
return -EIO;
 
str = next_string(skb);
if (!str)
return -EIO;
-   rate_up = simple_strtol(str, , 10);
-   if (*end)
+   err = kstrtoint(str, 10, _up);
+   if (err)
return -EIO;
 
state_str = next_string(skb);
@@ -417,7 +417,7 @@ static int process_command(struct solos_card *card, int 
port, struct sk_buff *sk
struct solos_param *prm;
unsigned long flags;
int cmdpid;
-   int found = 0;
+   int found = 0, err;
 
if (skb->len < 7)
return 0;
@@ -428,7 +428,9 @@ static int process_command(struct solos_card *card, int 
port, struct sk_buff *sk
skb->data[6] != '\n')
return 0;
 
-   cmdpid = simple_strtol(>data[1], NULL, 10);
+   err = kstrtoint(>data[1], 10, );
+   if (err)
+   return err;
 
spin_lock_irqsave(>param_queue_lock, flags);
list_for_each_entry(prm, >param_queue, list) {
-- 
2.4.10

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

[PATCH v3 3/5] stmmac: remove some __func__ printing

[LABBE Corentin]

Now that stmmac use netdev_xxx, some __func__ are not necessary since their
use was to clearly identify which driver was logging.
Moreover, as pointed by Joe Perches, using __func__ in xxx_dbg functions
is relatively low value and dynamic debug can add it.

This patch remove __func__ where such printing is useless.

Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 81 ++-
 1 file changed, 34 insertions(+), 47 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 2dc9260..f096416 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -448,8 +448,8 @@ static int stmmac_hwtstamp_ioctl(struct net_device *dev, 
struct ifreq *ifr)
   sizeof(struct hwtstamp_config)))
return -EFAULT;
 
-   netdev_dbg(priv->dev, "%s config flags:0x%x, tx_type:0x%x, 
rx_filter:0x%x\n",
-  __func__, config.flags, config.tx_type, config.rx_filter);
+   netdev_dbg(priv->dev, "config flags:0x%x, tx_type:0x%x, 
rx_filter:0x%x\n",
+  config.flags, config.tx_type, config.rx_filter);
 
/* reserved for future extensions */
if (config.flags)
@@ -833,8 +833,7 @@ static int stmmac_init_phy(struct net_device *dev)
 
snprintf(phy_id_fmt, MII_BUS_ID_SIZE + 3, PHY_ID_FMT, bus_id,
 priv->plat->phy_addr);
-   netdev_dbg(priv->dev, "%s: trying to attach to %s\n", __func__,
-  phy_id_fmt);
+   netdev_dbg(priv->dev, "trying to attach to %s\n", phy_id_fmt);
 
phydev = phy_connect(dev, phy_id_fmt, _adjust_link,
 interface);
@@ -991,8 +990,7 @@ static int stmmac_init_rx_buffers(struct stmmac_priv *priv, 
struct dma_desc *p,
 
skb = __netdev_alloc_skb_ip_align(priv->dev, priv->dma_buf_sz, flags);
if (!skb) {
-   netdev_err(priv->dev,
-  "%s: Rx init fails; skb is NULL\n", __func__);
+   netdev_err(priv->dev, "Rx init fails; skb is NULL\n");
return -ENOMEM;
}
priv->rx_skbuff[i] = skb;
@@ -1000,7 +998,7 @@ static int stmmac_init_rx_buffers(struct stmmac_priv 
*priv, struct dma_desc *p,
priv->dma_buf_sz,
DMA_FROM_DEVICE);
if (dma_mapping_error(priv->device, priv->rx_skbuff_dma[i])) {
-   netdev_err(priv->dev, "%s: DMA mapping error\n", __func__);
+   netdev_err(priv->dev, "DMA mapping error\n");
dev_kfree_skb_any(skb);
return -EINVAL;
}
@@ -1050,8 +1048,8 @@ static int init_dma_desc_rings(struct net_device *dev, 
gfp_t flags)
priv->dma_buf_sz = bfsize;
 
if (netif_msg_probe(priv))
-   netdev_dbg(priv->dev, "%s: txsize %d, rxsize %d, bfsize %d\n",
-  __func__, txsize, rxsize, bfsize);
+   netdev_dbg(priv->dev, "txsize %d, rxsize %d, bfsize %d\n",
+  txsize, rxsize, bfsize);
 
if (netif_msg_probe(priv)) {
netdev_dbg(priv->dev, "(%s) dma_rx_phy=0x%08x 
dma_tx_phy=0x%08x\n",
@@ -1357,8 +1355,8 @@ static void stmmac_tx_clean(struct stmmac_priv *priv)
stmmac_get_tx_hwtstamp(priv, entry, skb);
}
if (netif_msg_tx_done(priv))
-   netdev_dbg(priv->dev, "%s: curr %d, dirty %d\n",
-  __func__, priv->cur_tx, priv->dirty_tx);
+   netdev_dbg(priv->dev, "curr %d, dirty %d\n",
+  priv->cur_tx, priv->dirty_tx);
 
if (likely(priv->tx_skbuff_dma[entry].buf)) {
if (priv->tx_skbuff_dma[entry].map_as_page)
@@ -1396,8 +1394,7 @@ static void stmmac_tx_clean(struct stmmac_priv *priv)
if (netif_queue_stopped(priv->dev) &&
stmmac_tx_avail(priv) > STMMAC_TX_THRESH(priv)) {
if (netif_msg_tx_done(priv))
-   netdev_dbg(priv->dev, "%s: restart transmit\n",
-  __func__);
+   netdev_dbg(priv->dev, "restart transmit\n");
netif_wake_queue(priv->dev);
}
netif_tx_unlock(priv->dev);
@@ -1714,8 +1711,7 @@ static int stmmac_hw_setup(struct net_device *dev, bool 
init_ptp)
/* DMA initialization

[PATCH] net: stmmac: fix double-initialization of phy_iface

[LABBE Corentin]

The variable phy_iface is double-initialized to itself.
This patch remove that.

Reported-by: coverity (CID 1271141)
Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c 
b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
index 11baa4b..0cd3ecf 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
@@ -354,7 +354,7 @@ static int gmac_clk_init(struct rk_priv_data *bsp_priv)
 
 static int gmac_clk_enable(struct rk_priv_data *bsp_priv, bool enable)
 {
-   int phy_iface = phy_iface = bsp_priv->phy_iface;
+   int phy_iface = bsp_priv->phy_iface;
 
if (enable) {
if (!bsp_priv->clk_enabled) {
-- 
2.4.10

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

[PATCH v3] stmmac: improve logging

[LABBE Corentin]

Hello

This patch series try to improve logging of the stmmac driver.

Changes since v2
- Rollback to dev_ for some early init printing
- rebased on 4.4-rc1

Changes since v1
- Use netdev_xxx instead of dev_xxx
- Use netif_xxx instead of "if (netif_msg_type) dev_xxx"
Regards
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 1/5] stmmac: replace all pr_xxx by their netdev_xxx counterpart

[LABBE Corentin]

The stmmac driver use lots of pr_xxx functions to print information.
This is bad since we cannot know which device logs the information.
(moreover if two stmmac device are present)

Furthermore, it seems that it assumes wrongly that all logs will always
be subsequent by using a dev_xxx then some indented pr_xxx like this:
kernel: sun7i-dwmac 1c5.ethernet: no reset control found
kernel:  Ring mode enabled
kernel:  No HW DMA feature register supported
kernel:  Normal descriptors
kernel:  TX Checksum insertion supported

So this patch replace all pr_xxx by their netdev_xxx counterpart.
Excepts for some printing where netdev "cause" unpretty output like:
sun7i-dwmac 1c5.ethernet (unnamed net_device) (uninitialized): no reset 
control found
In those case, I keep dev_xxx.

In the same time I remove some "stmmac:" print since
this will be a duplicate with that dev_xxx displays.
And this patch also change some pr_info by netdev_err when
the word ERROR is printed.

Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 210 --
 drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c |  14 +-
 2 files changed, 123 insertions(+), 101 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 64d8aa4..a4788c5 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -298,7 +298,7 @@ bool stmmac_eee_init(struct stmmac_priv *priv)
 */
spin_lock_irqsave(>lock, flags);
if (priv->eee_active) {
-   pr_debug("stmmac: disable EEE\n");
+   netdev_dbg(priv->dev, "disable EEE\n");
del_timer_sync(>eee_ctrl_timer);
priv->hw->mac->set_eee_timer(priv->hw, 0,
 tx_lpi_timer);
@@ -327,7 +327,7 @@ bool stmmac_eee_init(struct stmmac_priv *priv)
ret = true;
spin_unlock_irqrestore(>lock, flags);
 
-   pr_debug("stmmac: Energy-Efficient Ethernet initialized\n");
+   netdev_dbg(priv->dev, "Energy-Efficient Ethernet 
initialized\n");
}
 out:
return ret;
@@ -448,8 +448,8 @@ static int stmmac_hwtstamp_ioctl(struct net_device *dev, 
struct ifreq *ifr)
   sizeof(struct hwtstamp_config)))
return -EFAULT;
 
-   pr_debug("%s config flags:0x%x, tx_type:0x%x, rx_filter:0x%x\n",
-__func__, config.flags, config.tx_type, config.rx_filter);
+   netdev_dbg(priv->dev, "%s config flags:0x%x, tx_type:0x%x, 
rx_filter:0x%x\n",
+  __func__, config.flags, config.tx_type, config.rx_filter);
 
/* reserved for future extensions */
if (config.flags)
@@ -659,10 +659,10 @@ static int stmmac_init_ptp(struct stmmac_priv *priv)
priv->adv_ts = 1;
 
if (netif_msg_hw(priv) && priv->dma_cap.time_stamp)
-   pr_debug("IEEE 1588-2002 Time Stamp supported\n");
+   netdev_dbg(priv->dev, "IEEE 1588-2002 Time Stamp supported\n");
 
if (netif_msg_hw(priv) && priv->adv_ts)
-   pr_debug("IEEE 1588-2008 Advanced Time Stamp supported\n");
+   netdev_dbg(priv->dev, "IEEE 1588-2008 Advanced Time Stamp 
supported\n");
 
priv->hw->ptp = _ptp;
priv->hwts_tx_en = 0;
@@ -742,8 +742,9 @@ static void stmmac_adjust_link(struct net_device *dev)
break;
default:
if (netif_msg_link(priv))
-   pr_warn("%s: Speed (%d) not 10/100\n",
-   dev->name, phydev->speed);
+   netdev_warn(priv->dev,
+   "Speed (%d) not 10/100\n",
+   phydev->speed);
break;
}
 
@@ -790,10 +791,10 @@ static void stmmac_check_pcs_mode(struct stmmac_priv 
*priv)
(interface == PHY_INTERFACE_MODE_RGMII_ID) ||
(interface == PHY_INTERFACE_MODE_RGMII_RXID) ||
(interface == PHY_INTERFACE_MODE_RGMII_TXID)) {
-   pr_debug("STMMAC: PCS RGMII support enable\n");
+   netdev_dbg(priv->dev, "PCS RGMII support enabled\n");
priv->pcs = STMMAC_PCS_RGMII;
 

[PATCH v3 4/5] stmmac: Fix simple style problem

[LABBE Corentin]

This patch fix the following warnings:
- braces {} should be used on all arms of this statement
- Prefer seq_puts to seq_printf
- No space is necessary after a cast
- Missing a blank line after declarations
- Please don't use multiple blank lines
- Comparison to NULL could be written
- networking block comments don't use an empty /* line
- Do not include the paragraph about writing to the Free Software

Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 71 +++
 drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c | 18 +++---
 2 files changed, 40 insertions(+), 49 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index f096416..d8adbdb 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -13,10 +13,6 @@
   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, write to the Free Software Foundation, Inc.,
-  51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
-
   The full GNU General Public License is included in this distribution in
   the file called "COPYING".
 
@@ -197,7 +193,7 @@ static void print_pkt(unsigned char *buf, int len)
 }
 
 /* minimum number of free TX descriptors required to wake up TX process */
-#define STMMAC_TX_THRESH(x)(x->dma_tx_size/4)
+#define STMMAC_TX_THRESH(x)(x->dma_tx_size / 4)
 
 static inline u32 stmmac_tx_avail(struct stmmac_priv *priv)
 {
@@ -207,7 +203,7 @@ static inline u32 stmmac_tx_avail(struct stmmac_priv *priv)
 /**
  * stmmac_hw_fix_mac_speed - callback for speed selection
  * @priv: driver private structure
- * Description: on some platforms (e.g. ST), some HW system configuraton
+ * Description: on some platforms (e.g. ST), some HW system configuration
  * registers have to be set according to the link speed negotiated.
  */
 static inline void stmmac_hw_fix_mac_speed(struct stmmac_priv *priv)
@@ -371,8 +367,6 @@ static void stmmac_get_tx_hwtstamp(struct stmmac_priv *priv,
shhwtstamp.hwtstamp = ns_to_ktime(ns);
/* pass tstamp to stack */
skb_tstamp_tx(skb, );
-
-   return;
 }
 
 /* stmmac_get_rx_hwtstamp - get HW RX timestamps
@@ -615,7 +609,7 @@ static int stmmac_hwtstamp_ioctl(struct net_device *dev, 
struct ifreq *ifr)
 * 2^x * y == (y << x), hence
 * 2^32 * 5000 ==> (5000 << 32)
 */
-   temp = (u64) (5000ULL << 32);
+   temp = (u64)(5000ULL << 32);
priv->default_addend = div_u64(temp, priv->clk_ptp_rate);
priv->hw->ptp->config_addend(priv->ioaddr,
 priv->default_addend);
@@ -695,7 +689,7 @@ static void stmmac_adjust_link(struct net_device *dev)
int new_state = 0;
unsigned int fc = priv->flow_ctrl, pause_time = priv->pause;
 
-   if (phydev == NULL)
+   if (!phydev)
return;
 
spin_lock_irqsave(>lock, flags);
@@ -704,7 +698,8 @@ static void stmmac_adjust_link(struct net_device *dev)
u32 ctrl = readl(priv->ioaddr + MAC_CTRL_REG);
 
/* Now we make sure that we can be in full duplex mode.
-* If not, we operate in half-duplex mode. */
+* If not, we operate in half-duplex mode.
+*/
if (phydev->duplex != priv->oldduplex) {
new_state = 1;
if (!(phydev->duplex))
@@ -730,11 +725,10 @@ static void stmmac_adjust_link(struct net_device *dev)
case 10:
if (priv->plat->has_gmac) {
ctrl |= priv->hw->link.port;
-   if (phydev->speed == SPEED_100) {
+   if (phydev->speed == SPEED_100)
ctrl |= priv->hw->link.speed;
-   } else {
+   else
ctrl &= ~(priv->hw->link.speed);
-   }
} else {
ctrl &= ~priv->hw->link.port;
}
@@ -816,6 +810,7 @@ static int stmmac_init_phy(struct net_device *dev)
char bus_id[MII_BUS_ID_SIZE];
int interface = priv->plat->interface;
int max_speed = priv->plat->max_speed;
+
priv->oldlink = 0;
priv->speed = 0;
priv-

[PATCH v3 5/5] net: stmmac: replace if (netif_msg_type) by their netif_xxx counterpart

[LABBE Corentin]

As sugested by Joe Perches, we could replace all
if (netif_msg_type(priv)) dev_xxx(priv->devices, ...)
by the simplier macro netif_xxx(priv, hw, priv->dev, ...)

Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 58 ++-
 1 file changed, 25 insertions(+), 33 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index d8adbdb..2a1aa73 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -652,11 +652,13 @@ static int stmmac_init_ptp(struct stmmac_priv *priv)
if (priv->dma_cap.atime_stamp && priv->extend_desc)
priv->adv_ts = 1;
 
-   if (netif_msg_hw(priv) && priv->dma_cap.time_stamp)
-   netdev_dbg(priv->dev, "IEEE 1588-2002 Time Stamp supported\n");
+   if (priv->dma_cap.time_stamp)
+   netif_dbg(priv, hw, priv->dev,
+ "IEEE 1588-2002 Time Stamp supported\n");
 
-   if (netif_msg_hw(priv) && priv->adv_ts)
-   netdev_dbg(priv->dev, "IEEE 1588-2008 Advanced Time Stamp 
supported\n");
+   if (priv->adv_ts)
+   netif_dbg(priv, hw, priv->dev,
+ "IEEE 1588-2008 Advanced Time Stamp supported\n");
 
priv->hw->ptp = _ptp;
priv->hwts_tx_en = 0;
@@ -735,10 +737,9 @@ static void stmmac_adjust_link(struct net_device *dev)
stmmac_hw_fix_mac_speed(priv);
break;
default:
-   if (netif_msg_link(priv))
-   netdev_warn(priv->dev,
-   "Speed (%d) not 10/100\n",
-   phydev->speed);
+   netif_warn(priv, link, priv->dev,
+  "Speed (%d) not 10/100\n",
+  phydev->speed);
break;
}
 
@@ -1041,18 +1042,15 @@ static int init_dma_desc_rings(struct net_device *dev, 
gfp_t flags)
 
priv->dma_buf_sz = bfsize;
 
-   if (netif_msg_probe(priv))
-   netdev_dbg(priv->dev, "txsize %d, rxsize %d, bfsize %d\n",
-  txsize, rxsize, bfsize);
+   netif_dbg(priv, probe, priv->dev, "txsize %d, rxsize %d, bfsize %d\n",
+ txsize, rxsize, bfsize);
 
-   if (netif_msg_probe(priv)) {
-   netdev_dbg(priv->dev, "(%s) dma_rx_phy=0x%08x 
dma_tx_phy=0x%08x\n",
-  __func__, (u32)priv->dma_rx_phy,
-  (u32)priv->dma_tx_phy);
+   netif_dbg(priv, probe, priv->dev, "(%s) dma_rx_phy=0x%08x 
dma_tx_phy=0x%08x\n",
+ __func__, (u32)priv->dma_rx_phy, (u32)priv->dma_tx_phy);
+
+   /* RX INITIALIZATION */
+   netif_dbg(priv, probe, priv->dev, "SKB addresses:\nskb\t\tskb data\tdma 
data\n");
 
-   /* RX INITIALIZATION */
-   netdev_dbg(priv->dev, "SKB addresses:\nskb\t\tskb data\tdma 
data\n");
-   }
for (i = 0; i < rxsize; i++) {
struct dma_desc *p;
 
@@ -1065,11 +1063,9 @@ static int init_dma_desc_rings(struct net_device *dev, 
gfp_t flags)
if (ret)
goto err_init_rx_buffers;
 
-   if (netif_msg_probe(priv))
-   netdev_dbg(priv->dev, "[%p]\t[%p]\t[%x]\n",
-  priv->rx_skbuff[i],
-priv->rx_skbuff[i]->data,
-(unsigned int)priv->rx_skbuff_dma[i]);
+   netif_dbg(priv, probe, priv->dev, "[%p]\t[%p]\t[%x]\n",
+ priv->rx_skbuff[i], priv->rx_skbuff[i]->data,
+ (unsigned int)priv->rx_skbuff_dma[i]);
}
priv->cur_rx = 0;
priv->dirty_rx = (unsigned int)(i - rxsize);
@@ -1349,9 +1345,8 @@ static void stmmac_tx_clean(struct stmmac_priv *priv)
 
stmmac_get_tx_hwtstamp(priv, entry, skb);
}
-   if (netif_msg_tx_done(priv))
-   netdev_dbg(priv->dev, "curr %d, dirty %d\n",
-  priv->cur_tx, priv->dirty_tx);
+   netif_dbg(priv, tx_done, priv->dev, "curr %d, dirty %d\n",
+ priv->cur_tx, priv->dirty_tx);
 
if (likely(priv->tx_skbuff_dma[entry].buf)) {
 

[PATCH v3 2/5] stmmac: replace hardcoded function name by __func__

[LABBE Corentin]

Some printing have the function name hardcoded.
It is better to use __func__ instead.

Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index a4788c5..2dc9260 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -833,7 +833,7 @@ static int stmmac_init_phy(struct net_device *dev)
 
snprintf(phy_id_fmt, MII_BUS_ID_SIZE + 3, PHY_ID_FMT, bus_id,
 priv->plat->phy_addr);
-   netdev_dbg(priv->dev, "stmmac_init_phy: trying to attach to 
%s\n",
+   netdev_dbg(priv->dev, "%s: trying to attach to %s\n", __func__,
   phy_id_fmt);
 
phydev = phy_connect(dev, phy_id_fmt, _adjust_link,
@@ -866,8 +866,8 @@ static int stmmac_init_phy(struct net_device *dev)
phy_disconnect(phydev);
return -ENODEV;
}
-   netdev_dbg(priv->dev, "stmmac_init_phy: attached to PHY (UID 0x%x) Link 
= %d\n",
-  phydev->phy_id, phydev->link);
+   netdev_dbg(priv->dev, "%s: attached to PHY (UID 0x%x) Link = %d\n",
+  __func__, phydev->phy_id, phydev->link);
 
priv->phydev = phydev;
 
-- 
2.4.10

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

[PATCH v4 5/5] net: stmmac: replace if (netif_msg_type) by their netif_xxx counterpart

[LABBE Corentin]

As sugested by Joe Perches, we could replace all
if (netif_msg_type(priv)) dev_xxx(priv->devices, ...)
by the simplier macro netif_xxx(priv, hw, priv->dev, ...)

Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 58 ++-
 1 file changed, 25 insertions(+), 33 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index d8adbdb..2a1aa73 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -652,11 +652,13 @@ static int stmmac_init_ptp(struct stmmac_priv *priv)
if (priv->dma_cap.atime_stamp && priv->extend_desc)
priv->adv_ts = 1;
 
-   if (netif_msg_hw(priv) && priv->dma_cap.time_stamp)
-   netdev_dbg(priv->dev, "IEEE 1588-2002 Time Stamp supported\n");
+   if (priv->dma_cap.time_stamp)
+   netif_dbg(priv, hw, priv->dev,
+ "IEEE 1588-2002 Time Stamp supported\n");
 
-   if (netif_msg_hw(priv) && priv->adv_ts)
-   netdev_dbg(priv->dev, "IEEE 1588-2008 Advanced Time Stamp 
supported\n");
+   if (priv->adv_ts)
+   netif_dbg(priv, hw, priv->dev,
+ "IEEE 1588-2008 Advanced Time Stamp supported\n");
 
priv->hw->ptp = _ptp;
priv->hwts_tx_en = 0;
@@ -735,10 +737,9 @@ static void stmmac_adjust_link(struct net_device *dev)
stmmac_hw_fix_mac_speed(priv);
break;
default:
-   if (netif_msg_link(priv))
-   netdev_warn(priv->dev,
-   "Speed (%d) not 10/100\n",
-   phydev->speed);
+   netif_warn(priv, link, priv->dev,
+  "Speed (%d) not 10/100\n",
+  phydev->speed);
break;
}
 
@@ -1041,18 +1042,15 @@ static int init_dma_desc_rings(struct net_device *dev, 
gfp_t flags)
 
priv->dma_buf_sz = bfsize;
 
-   if (netif_msg_probe(priv))
-   netdev_dbg(priv->dev, "txsize %d, rxsize %d, bfsize %d\n",
-  txsize, rxsize, bfsize);
+   netif_dbg(priv, probe, priv->dev, "txsize %d, rxsize %d, bfsize %d\n",
+ txsize, rxsize, bfsize);
 
-   if (netif_msg_probe(priv)) {
-   netdev_dbg(priv->dev, "(%s) dma_rx_phy=0x%08x 
dma_tx_phy=0x%08x\n",
-  __func__, (u32)priv->dma_rx_phy,
-  (u32)priv->dma_tx_phy);
+   netif_dbg(priv, probe, priv->dev, "(%s) dma_rx_phy=0x%08x 
dma_tx_phy=0x%08x\n",
+ __func__, (u32)priv->dma_rx_phy, (u32)priv->dma_tx_phy);
+
+   /* RX INITIALIZATION */
+   netif_dbg(priv, probe, priv->dev, "SKB addresses:\nskb\t\tskb data\tdma 
data\n");
 
-   /* RX INITIALIZATION */
-   netdev_dbg(priv->dev, "SKB addresses:\nskb\t\tskb data\tdma 
data\n");
-   }
for (i = 0; i < rxsize; i++) {
struct dma_desc *p;
 
@@ -1065,11 +1063,9 @@ static int init_dma_desc_rings(struct net_device *dev, 
gfp_t flags)
if (ret)
goto err_init_rx_buffers;
 
-   if (netif_msg_probe(priv))
-   netdev_dbg(priv->dev, "[%p]\t[%p]\t[%x]\n",
-  priv->rx_skbuff[i],
-priv->rx_skbuff[i]->data,
-(unsigned int)priv->rx_skbuff_dma[i]);
+   netif_dbg(priv, probe, priv->dev, "[%p]\t[%p]\t[%x]\n",
+ priv->rx_skbuff[i], priv->rx_skbuff[i]->data,
+ (unsigned int)priv->rx_skbuff_dma[i]);
}
priv->cur_rx = 0;
priv->dirty_rx = (unsigned int)(i - rxsize);
@@ -1349,9 +1345,8 @@ static void stmmac_tx_clean(struct stmmac_priv *priv)
 
stmmac_get_tx_hwtstamp(priv, entry, skb);
}
-   if (netif_msg_tx_done(priv))
-   netdev_dbg(priv->dev, "curr %d, dirty %d\n",
-  priv->cur_tx, priv->dirty_tx);
+   netif_dbg(priv, tx_done, priv->dev, "curr %d, dirty %d\n",
+ priv->cur_tx, priv->dirty_tx);
 
if (likely(priv->tx_skbuff_dma[entry].buf)) {
 

[PATCH v4 1/5] net: stmmac: replace all pr_xxx by their netdev_xxx counterpart

[LABBE Corentin]

The stmmac driver use lots of pr_xxx functions to print information.
This is bad since we cannot know which device logs the information.
(moreover if two stmmac device are present)

Furthermore, it seems that it assumes wrongly that all logs will always
be subsequent by using a dev_xxx then some indented pr_xxx like this:
kernel: sun7i-dwmac 1c5.ethernet: no reset control found
kernel:  Ring mode enabled
kernel:  No HW DMA feature register supported
kernel:  Normal descriptors
kernel:  TX Checksum insertion supported

So this patch replace all pr_xxx by their netdev_xxx counterpart.
Excepts for some printing where netdev "cause" unpretty output like:
sun7i-dwmac 1c5.ethernet (unnamed net_device) (uninitialized): no reset 
control found
In those case, I keep dev_xxx.

In the same time I remove some "stmmac:" print since
this will be a duplicate with that dev_xxx displays.
And this patch also change some pr_info by netdev_err when
the word ERROR is printed.

Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 210 --
 drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c |  14 +-
 2 files changed, 123 insertions(+), 101 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 64d8aa4..633a47d 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -298,7 +298,7 @@ bool stmmac_eee_init(struct stmmac_priv *priv)
 */
spin_lock_irqsave(>lock, flags);
if (priv->eee_active) {
-   pr_debug("stmmac: disable EEE\n");
+   netdev_dbg(priv->dev, "disable EEE\n");
del_timer_sync(>eee_ctrl_timer);
priv->hw->mac->set_eee_timer(priv->hw, 0,
 tx_lpi_timer);
@@ -327,7 +327,7 @@ bool stmmac_eee_init(struct stmmac_priv *priv)
ret = true;
spin_unlock_irqrestore(>lock, flags);
 
-   pr_debug("stmmac: Energy-Efficient Ethernet initialized\n");
+   netdev_dbg(priv->dev, "Energy-Efficient Ethernet 
initialized\n");
}
 out:
return ret;
@@ -448,8 +448,8 @@ static int stmmac_hwtstamp_ioctl(struct net_device *dev, 
struct ifreq *ifr)
   sizeof(struct hwtstamp_config)))
return -EFAULT;
 
-   pr_debug("%s config flags:0x%x, tx_type:0x%x, rx_filter:0x%x\n",
-__func__, config.flags, config.tx_type, config.rx_filter);
+   netdev_dbg(priv->dev, "%s config flags:0x%x, tx_type:0x%x, 
rx_filter:0x%x\n",
+  __func__, config.flags, config.tx_type, config.rx_filter);
 
/* reserved for future extensions */
if (config.flags)
@@ -659,10 +659,10 @@ static int stmmac_init_ptp(struct stmmac_priv *priv)
priv->adv_ts = 1;
 
if (netif_msg_hw(priv) && priv->dma_cap.time_stamp)
-   pr_debug("IEEE 1588-2002 Time Stamp supported\n");
+   netdev_dbg(priv->dev, "IEEE 1588-2002 Time Stamp supported\n");
 
if (netif_msg_hw(priv) && priv->adv_ts)
-   pr_debug("IEEE 1588-2008 Advanced Time Stamp supported\n");
+   netdev_dbg(priv->dev, "IEEE 1588-2008 Advanced Time Stamp 
supported\n");
 
priv->hw->ptp = _ptp;
priv->hwts_tx_en = 0;
@@ -742,8 +742,9 @@ static void stmmac_adjust_link(struct net_device *dev)
break;
default:
if (netif_msg_link(priv))
-   pr_warn("%s: Speed (%d) not 10/100\n",
-   dev->name, phydev->speed);
+   netdev_warn(priv->dev,
+   "Speed (%d) not 10/100\n",
+   phydev->speed);
break;
}
 
@@ -790,10 +791,10 @@ static void stmmac_check_pcs_mode(struct stmmac_priv 
*priv)
(interface == PHY_INTERFACE_MODE_RGMII_ID) ||
(interface == PHY_INTERFACE_MODE_RGMII_RXID) ||
(interface == PHY_INTERFACE_MODE_RGMII_TXID)) {
-   pr_debug("STMMAC: PCS RGMII support enable\n");
+   netdev_dbg(priv->dev, "PCS RGMII support enabled\n");
priv->pcs = STMMAC_PCS_RGMII;
 

[PATCH v4 2/5] net: stmmac: replace hardcoded function name by __func__

[LABBE Corentin]

Some printing have the function name hardcoded.
It is better to use __func__ instead.

Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 633a47d..911cf28 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -833,7 +833,7 @@ static int stmmac_init_phy(struct net_device *dev)
 
snprintf(phy_id_fmt, MII_BUS_ID_SIZE + 3, PHY_ID_FMT, bus_id,
 priv->plat->phy_addr);
-   netdev_dbg(priv->dev, "stmmac_init_phy: trying to attach to 
%s\n",
+   netdev_dbg(priv->dev, "%s: trying to attach to %s\n", __func__,
   phy_id_fmt);
 
phydev = phy_connect(dev, phy_id_fmt, _adjust_link,
@@ -866,8 +866,8 @@ static int stmmac_init_phy(struct net_device *dev)
phy_disconnect(phydev);
return -ENODEV;
}
-   netdev_dbg(priv->dev, "stmmac_init_phy: attached to PHY (UID 0x%x) Link 
= %d\n",
-  phydev->phy_id, phydev->link);
+   netdev_dbg(priv->dev, "%s: attached to PHY (UID 0x%x) Link = %d\n",
+  __func__, phydev->phy_id, phydev->link);
 
priv->phydev = phydev;
 
-- 
2.4.10

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

[PATCH v4 3/5] net: stmmac: remove some __func__ printing

[LABBE Corentin]

Now that stmmac use netdev_xxx, some __func__ are not necessary since their
use was to clearly identify which driver was logging.
Moreover, as pointed by Joe Perches, using __func__ in xxx_dbg functions
is relatively low value and dynamic debug can add it.

This patch remove __func__ where such printing is useless.

Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 81 ++-
 1 file changed, 34 insertions(+), 47 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 911cf28..f096416 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -448,8 +448,8 @@ static int stmmac_hwtstamp_ioctl(struct net_device *dev, 
struct ifreq *ifr)
   sizeof(struct hwtstamp_config)))
return -EFAULT;
 
-   netdev_dbg(priv->dev, "%s config flags:0x%x, tx_type:0x%x, 
rx_filter:0x%x\n",
-  __func__, config.flags, config.tx_type, config.rx_filter);
+   netdev_dbg(priv->dev, "config flags:0x%x, tx_type:0x%x, 
rx_filter:0x%x\n",
+  config.flags, config.tx_type, config.rx_filter);
 
/* reserved for future extensions */
if (config.flags)
@@ -833,8 +833,7 @@ static int stmmac_init_phy(struct net_device *dev)
 
snprintf(phy_id_fmt, MII_BUS_ID_SIZE + 3, PHY_ID_FMT, bus_id,
 priv->plat->phy_addr);
-   netdev_dbg(priv->dev, "%s: trying to attach to %s\n", __func__,
-  phy_id_fmt);
+   netdev_dbg(priv->dev, "trying to attach to %s\n", phy_id_fmt);
 
phydev = phy_connect(dev, phy_id_fmt, _adjust_link,
 interface);
@@ -991,8 +990,7 @@ static int stmmac_init_rx_buffers(struct stmmac_priv *priv, 
struct dma_desc *p,
 
skb = __netdev_alloc_skb_ip_align(priv->dev, priv->dma_buf_sz, flags);
if (!skb) {
-   netdev_err(priv->dev,
-  "%s: Rx init fails; skb is NULL\n", __func__);
+   netdev_err(priv->dev, "Rx init fails; skb is NULL\n");
return -ENOMEM;
}
priv->rx_skbuff[i] = skb;
@@ -1000,7 +998,7 @@ static int stmmac_init_rx_buffers(struct stmmac_priv 
*priv, struct dma_desc *p,
priv->dma_buf_sz,
DMA_FROM_DEVICE);
if (dma_mapping_error(priv->device, priv->rx_skbuff_dma[i])) {
-   netdev_err(priv->dev, "%s: DMA mapping error\n", __func__);
+   netdev_err(priv->dev, "DMA mapping error\n");
dev_kfree_skb_any(skb);
return -EINVAL;
}
@@ -1050,8 +1048,8 @@ static int init_dma_desc_rings(struct net_device *dev, 
gfp_t flags)
priv->dma_buf_sz = bfsize;
 
if (netif_msg_probe(priv))
-   netdev_dbg(priv->dev, "%s: txsize %d, rxsize %d, bfsize %d\n",
-  __func__, txsize, rxsize, bfsize);
+   netdev_dbg(priv->dev, "txsize %d, rxsize %d, bfsize %d\n",
+  txsize, rxsize, bfsize);
 
if (netif_msg_probe(priv)) {
netdev_dbg(priv->dev, "(%s) dma_rx_phy=0x%08x 
dma_tx_phy=0x%08x\n",
@@ -1357,8 +1355,8 @@ static void stmmac_tx_clean(struct stmmac_priv *priv)
stmmac_get_tx_hwtstamp(priv, entry, skb);
}
if (netif_msg_tx_done(priv))
-   netdev_dbg(priv->dev, "%s: curr %d, dirty %d\n",
-  __func__, priv->cur_tx, priv->dirty_tx);
+   netdev_dbg(priv->dev, "curr %d, dirty %d\n",
+  priv->cur_tx, priv->dirty_tx);
 
if (likely(priv->tx_skbuff_dma[entry].buf)) {
if (priv->tx_skbuff_dma[entry].map_as_page)
@@ -1396,8 +1394,7 @@ static void stmmac_tx_clean(struct stmmac_priv *priv)
if (netif_queue_stopped(priv->dev) &&
stmmac_tx_avail(priv) > STMMAC_TX_THRESH(priv)) {
if (netif_msg_tx_done(priv))
-   netdev_dbg(priv->dev, "%s: restart transmit\n",
-  __func__);
+   netdev_dbg(priv->dev, "restart transmit\n");
netif_wake_queue(priv->dev);
}
netif_tx_unlock(priv->dev);
@@ -1714,8 +1711,7 @@ static int stmmac_hw_setup(struct net_device *dev, bool 
init_ptp)
/* DMA initialization

[PATCH v4] stmmac: improve logging

[LABBE Corentin]

Hello

This patch series try to improve logging of the stmmac driver.

Changes since v3
- Fix a missing comma for letting patch to be used atomaticly.

Changes since v2
- Rollback to dev_ for some early init printing
- rebased on 4.4-rc1

Changes since v1
- Use netdev_xxx instead of dev_xxx
- Use netif_xxx instead of "if (netif_msg_type) dev_xxx"
Regards

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

Re: [PATCH v3 1/5] stmmac: replace all pr_xxx by their netdev_xxx counterpart

[LABBE Corentin]

On Thu, Nov 05, 2015 at 06:58:17PM +0800, kbuild test robot wrote:
> Hi LABBE,
> 
> [auto build test ERROR on: net/master]
> [also build test ERROR on: v4.3 next-20151105]
> 
> url:
> https://github.com/0day-ci/linux/commits/LABBE-Corentin/stmmac-replace-all-pr_xxx-by-their-netdev_xxx-counterpart/20151105-163344
> config: m68k-allyesconfig (attached as .config)
> reproduce:
> wget 
> https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
>  -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # save the attached .config to linux build tree
> make.cross ARCH=m68k 
> 
> Note: the 
> linux-review/LABBE-Corentin/stmmac-replace-all-pr_xxx-by-their-netdev_xxx-counterpart/20151105-163344
>  HEAD 6d5b68276276235c73d865a816807f6073f341e9 builds fine.
>   It only hurts bisectibility.
> 
> All error/warnings (new ones prefixed by >>):
> 
>drivers/net/ethernet/stmicro/stmmac/stmmac_main.c: In function 
> 'stmmac_open':
> >> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:1822:7: error: expected 
> >> ')' before '__func__'
>   __func__);
>   ^
> >> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:1822:7: warning: format 
> >> '%s' expects a matching 'char *' argument [-Wformat=]
> 
> vim +1822 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> 
>   1816priv->dma_rx_size = STMMAC_ALIGN(dma_rxsize);
>   1817priv->dma_buf_sz = STMMAC_ALIGN(buf_sz);
>   1818
>   1819ret = alloc_dma_desc_resources(priv);
>   1820if (ret < 0) {
>   1821netdev_err(priv->dev, "%s: DMA descriptors 
> allocation failed\n"
> > 1822   __func__);
>   1823goto dma_desc_error;
>   1824}
>   1825
> 
> ---
> 0-DAY kernel test infrastructureOpen Source Technology Center
> https://lists.01.org/pipermail/kbuild-all   Intel Corporation

This part is removed by subsequent patch.
I will send a fixed version.

Regards

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

[PATCH v4 4/5] net: stmmac: Fix simple style problem

[LABBE Corentin]

This patch fix the following warnings:
- braces {} should be used on all arms of this statement
- Prefer seq_puts to seq_printf
- No space is necessary after a cast
- Missing a blank line after declarations
- Please don't use multiple blank lines
- Comparison to NULL could be written
- networking block comments don't use an empty /* line
- Do not include the paragraph about writing to the Free Software

Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 71 +++
 drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c | 18 +++---
 2 files changed, 40 insertions(+), 49 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index f096416..d8adbdb 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -13,10 +13,6 @@
   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, write to the Free Software Foundation, Inc.,
-  51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
-
   The full GNU General Public License is included in this distribution in
   the file called "COPYING".
 
@@ -197,7 +193,7 @@ static void print_pkt(unsigned char *buf, int len)
 }
 
 /* minimum number of free TX descriptors required to wake up TX process */
-#define STMMAC_TX_THRESH(x)(x->dma_tx_size/4)
+#define STMMAC_TX_THRESH(x)(x->dma_tx_size / 4)
 
 static inline u32 stmmac_tx_avail(struct stmmac_priv *priv)
 {
@@ -207,7 +203,7 @@ static inline u32 stmmac_tx_avail(struct stmmac_priv *priv)
 /**
  * stmmac_hw_fix_mac_speed - callback for speed selection
  * @priv: driver private structure
- * Description: on some platforms (e.g. ST), some HW system configuraton
+ * Description: on some platforms (e.g. ST), some HW system configuration
  * registers have to be set according to the link speed negotiated.
  */
 static inline void stmmac_hw_fix_mac_speed(struct stmmac_priv *priv)
@@ -371,8 +367,6 @@ static void stmmac_get_tx_hwtstamp(struct stmmac_priv *priv,
shhwtstamp.hwtstamp = ns_to_ktime(ns);
/* pass tstamp to stack */
skb_tstamp_tx(skb, );
-
-   return;
 }
 
 /* stmmac_get_rx_hwtstamp - get HW RX timestamps
@@ -615,7 +609,7 @@ static int stmmac_hwtstamp_ioctl(struct net_device *dev, 
struct ifreq *ifr)
 * 2^x * y == (y << x), hence
 * 2^32 * 5000 ==> (5000 << 32)
 */
-   temp = (u64) (5000ULL << 32);
+   temp = (u64)(5000ULL << 32);
priv->default_addend = div_u64(temp, priv->clk_ptp_rate);
priv->hw->ptp->config_addend(priv->ioaddr,
 priv->default_addend);
@@ -695,7 +689,7 @@ static void stmmac_adjust_link(struct net_device *dev)
int new_state = 0;
unsigned int fc = priv->flow_ctrl, pause_time = priv->pause;
 
-   if (phydev == NULL)
+   if (!phydev)
return;
 
spin_lock_irqsave(>lock, flags);
@@ -704,7 +698,8 @@ static void stmmac_adjust_link(struct net_device *dev)
u32 ctrl = readl(priv->ioaddr + MAC_CTRL_REG);
 
/* Now we make sure that we can be in full duplex mode.
-* If not, we operate in half-duplex mode. */
+* If not, we operate in half-duplex mode.
+*/
if (phydev->duplex != priv->oldduplex) {
new_state = 1;
if (!(phydev->duplex))
@@ -730,11 +725,10 @@ static void stmmac_adjust_link(struct net_device *dev)
case 10:
if (priv->plat->has_gmac) {
ctrl |= priv->hw->link.port;
-   if (phydev->speed == SPEED_100) {
+   if (phydev->speed == SPEED_100)
ctrl |= priv->hw->link.speed;
-   } else {
+   else
ctrl &= ~(priv->hw->link.speed);
-   }
} else {
ctrl &= ~priv->hw->link.port;
}
@@ -816,6 +810,7 @@ static int stmmac_init_phy(struct net_device *dev)
char bus_id[MII_BUS_ID_SIZE];
int interface = priv->plat->interface;
int max_speed = priv->plat->max_speed;
+
priv->oldlink = 0;
priv->speed = 0;
priv-

[PATCH] net: stmmac: remove unneeded phy_iface variable

[LABBE Corentin]

The variable phy_iface is double-initialized and finally is not necessary
at all.

Reported-by: coverity (CID 1271141)
Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c 
b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
index 11baa4b..26a11b7 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
@@ -354,11 +354,9 @@ static int gmac_clk_init(struct rk_priv_data *bsp_priv)
 
 static int gmac_clk_enable(struct rk_priv_data *bsp_priv, bool enable)
 {
-   int phy_iface = phy_iface = bsp_priv->phy_iface;
-
if (enable) {
if (!bsp_priv->clk_enabled) {
-   if (phy_iface == PHY_INTERFACE_MODE_RMII) {
+   if (bsp_priv->phy_iface == PHY_INTERFACE_MODE_RMII) {
if (!IS_ERR(bsp_priv->mac_clk_rx))
clk_prepare_enable(
bsp_priv->mac_clk_rx);
@@ -390,7 +388,7 @@ static int gmac_clk_enable(struct rk_priv_data *bsp_priv, 
bool enable)
}
} else {
if (bsp_priv->clk_enabled) {
-   if (phy_iface == PHY_INTERFACE_MODE_RMII) {
+   if (bsp_priv->phy_iface == PHY_INTERFACE_MODE_RMII) {
if (!IS_ERR(bsp_priv->mac_clk_rx))
clk_disable_unprepare(
bsp_priv->mac_clk_rx);
-- 
2.4.10

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

[PATCH 04/11] net: packet: change vnet_hdr_len from int to size_t

[LABBE Corentin]

vnet_hdr_len cannot be negative and is use in operation/function that
wait for unsigned value.
This patch set vnet_hdr_len as size_t.

Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
---
 net/packet/af_packet.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index aa4b15c..58a5c8f 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -2632,7 +2632,7 @@ static int packet_snd(struct socket *sock, struct msghdr 
*msg, size_t len)
int err, reserve = 0;
struct virtio_net_hdr vnet_hdr = { 0 };
int offset = 0;
-   int vnet_hdr_len;
+   size_t vnet_hdr_len;
struct packet_sock *po = pkt_sk(sk);
unsigned short gso_type = 0;
int hlen, tlen;
@@ -3106,7 +3106,7 @@ static int packet_recvmsg(struct socket *sock, struct 
msghdr *msg, size_t len,
struct sock *sk = sock->sk;
struct sk_buff *skb;
int copied, err;
-   int vnet_hdr_len = 0;
+   size_t vnet_hdr_len = 0;
unsigned int origlen = 0;
 
err = -EINVAL;
-- 
2.4.10

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

[PATCH 06/11] net: ipv6: set the length parameter of rawv6_send_hdrinc() to size_t

[LABBE Corentin]

length is used in operation/function that wait for unsigned value.
Furthermore the only one call of rawv6_send_hdrinc() give a size_t as
length arguments.
So the parameter need to be set as size_t.

Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
---
 net/ipv6/raw.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv6/raw.c b/net/ipv6/raw.c
index fdbada156..5d9404b 100644
--- a/net/ipv6/raw.c
+++ b/net/ipv6/raw.c
@@ -609,7 +609,7 @@ out:
return err;
 }
 
-static int rawv6_send_hdrinc(struct sock *sk, struct msghdr *msg, int length,
+static int rawv6_send_hdrinc(struct sock *sk, struct msghdr *msg, size_t 
length,
struct flowi6 *fl6, struct dst_entry **dstp,
unsigned int flags)
 {
-- 
2.4.10

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

[PATCH 09/11] net: ipv6: hlen could be set as size_t

[LABBE Corentin]

The hlen member of raw6_frag_vec is used in operation/function that
wait for unsigned value. So it need to be set as size_t.

This patch do the same for the hlen variable.

Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
---
 net/ipv6/raw.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/ipv6/raw.c b/net/ipv6/raw.c
index 5d9404b..434e9ad 100644
--- a/net/ipv6/raw.c
+++ b/net/ipv6/raw.c
@@ -618,7 +618,7 @@ static int rawv6_send_hdrinc(struct sock *sk, struct msghdr 
*msg, size_t length,
struct sk_buff *skb;
int err;
struct rt6_info *rt = (struct rt6_info *)*dstp;
-   int hlen = LL_RESERVED_SPACE(rt->dst.dev);
+   size_t hlen = LL_RESERVED_SPACE(rt->dst.dev);
int tlen = rt->dst.dev->needed_tailroom;
 
if (length > rt->dst.dev->mtu) {
@@ -674,7 +674,7 @@ error:
 
 struct raw6_frag_vec {
struct msghdr *msg;
-   int hlen;
+   size_t hlen;
char c[4];
 };
 
-- 
2.4.10

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

[PATCH 07/11] net: caif: change chunk from int to size_t

[LABBE Corentin]

chunk cannot be negative and is use in operation/function that
wait for unsigned value. This patch set it as size_t.
The patch do the same for the size variable.

Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
---
 net/caif/caif_socket.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/caif/caif_socket.c b/net/caif/caif_socket.c
index cc85891..362f6d8 100644
--- a/net/caif/caif_socket.c
+++ b/net/caif/caif_socket.c
@@ -370,7 +370,7 @@ static int caif_stream_recvmsg(struct socket *sock, struct 
msghdr *msg,
timeo = sock_rcvtimeo(sk, flags_DONTWAIT);
 
do {
-   int chunk;
+   size_t chunk;
struct sk_buff *skb;
 
lock_sock(sk);
@@ -595,7 +595,8 @@ static int caif_stream_sendmsg(struct socket *sock, struct 
msghdr *msg,
 {
struct sock *sk = sock->sk;
struct caifsock *cf_sk = container_of(sk, struct caifsock, sk);
-   int err, size;
+   int err;
+   size_t size;
struct sk_buff *skb;
int sent = 0;
long timeo;
-- 
2.4.10

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

[PATCH 08/11] net: bluetooth: change the len parameter of sco_send_frame() to size_t

[LABBE Corentin]

len is used in operation/function that wait for unsigned value.
Furthermore the only one call of sco_send_frame give a size_t as argument.
So the parameter need to be set as size_t.

Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
---
 net/bluetooth/sco.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index f315c8d..1ae4b36 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -268,7 +268,7 @@ done:
return err;
 }
 
-static int sco_send_frame(struct sock *sk, struct msghdr *msg, int len)
+static int sco_send_frame(struct sock *sk, struct msghdr *msg, size_t len)
 {
struct sco_conn *conn = sco_pi(sk)->conn;
struct sk_buff *skb;
-- 
2.4.10

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

Re: [PATCH 10/11] net: ipv4: hlen could be set as size_t

[LABBE Corentin]

On Fri, Oct 23, 2015 at 06:08:52AM -0700, David Miller wrote:
> From: LABBE Corentin <clabbe.montj...@gmail.com>
> Date: Fri, 23 Oct 2015 14:10:35 +0200
> 
> > The hlen member of raw_frag_vec is used in operation/function that wait
> > for unsigned value. So it need to be set as size_t.
> > 
> > Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
> 
> You really need to test your changes.
> 
> Particularly on a platform where size_t has a different size than
> 'int'.  That will generate several warnings, as shown by all of the
> build warning reports sent by the kbuild test robot in response to
> this patch series.
> 
> Also, you need to CC: all relevant mailing lists for the entire series
> of patches, and also provide a "Subject: [PATCH 0/N] xxx" posting
> describing at a high level and in detail what this patch series is
> doing, and why.
> 
> It should also clearly state what exact tree you expect these patches
> to be merged into.
> 
> Thanks.

Hello

I am sorry, I have badly checked my build logs.

I have added all mailing list given by get_maintainer but this was done per 
patch with --to-cmd/--cc-cmd
Does it is better to put all mailing list globally ?

For the (0/N) mail, I forgot to add a subject on it 
https://lkml.org/lkml/2015/10/23/219

I will fix all minor finding and resend a proper version.

Thanks

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

[PATCH 03/11] net: llc: change copied to size_t in llc_ui_sendmsg

[LABBE Corentin]

The variable copied in llc_ui_sendmsg() cannot be negative and is used
in functions that wait for unsigned value, so set it as size_t (like it
is in llc_ui_recvmsg())

Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
---
 net/llc/af_llc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/llc/af_llc.c b/net/llc/af_llc.c
index 79b915d..f435b77 100644
--- a/net/llc/af_llc.c
+++ b/net/llc/af_llc.c
@@ -891,7 +891,8 @@ static int llc_ui_sendmsg(struct socket *sock, struct 
msghdr *msg, size_t len)
int noblock = flags & MSG_DONTWAIT;
struct sk_buff *skb;
size_t size = 0;
-   int rc = -EINVAL, copied = 0, hdrlen;
+   int rc = -EINVAL, hdrlen;
+   size_t copied = 0;
 
dprintk("%s: sending from %02X to %02X\n", __func__,
llc->laddr.lsap, llc->daddr.lsap);
-- 
2.4.10

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

[PATCH 02/11] net: llc: fix a setting of error value to size_t

[LABBE Corentin]

The variable copied is a size_t, so setting a negative value to it is
invalid.
The patch add an "err" variable for getting the error code.

Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
---
 net/llc/af_llc.c | 20 
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/net/llc/af_llc.c b/net/llc/af_llc.c
index 8dab4e5..79b915d 100644
--- a/net/llc/af_llc.c
+++ b/net/llc/af_llc.c
@@ -719,9 +719,10 @@ static int llc_ui_recvmsg(struct socket *sock, struct 
msghdr *msg, size_t len,
unsigned long used;
int target; /* Read at least this many bytes */
long timeo;
+   int err = 0;
 
lock_sock(sk);
-   copied = -ENOTCONN;
+   err = -ENOTCONN;
if (unlikely(sk->sk_type == SOCK_STREAM && sk->sk_state == TCP_LISTEN))
goto out;
 
@@ -745,9 +746,8 @@ static int llc_ui_recvmsg(struct socket *sock, struct 
msghdr *msg, size_t len,
 * and move it down to the bottom of the loop
 */
if (signal_pending(current)) {
-   if (copied)
-   break;
-   copied = timeo ? sock_intr_errno(timeo) : -EAGAIN;
+   if (!copied)
+   err = timeo ? sock_intr_errno(timeo) : -EAGAIN;
break;
}
 
@@ -775,7 +775,7 @@ static int llc_ui_recvmsg(struct socket *sock, struct 
msghdr *msg, size_t len,
break;
 
if (sk->sk_err) {
-   copied = sock_error(sk);
+   err = sock_error(sk);
break;
}
if (sk->sk_shutdown & RCV_SHUTDOWN)
@@ -787,13 +787,15 @@ static int llc_ui_recvmsg(struct socket *sock, struct 
msghdr *msg, size_t len,
 * This occurs when user tries to read
 * from never connected socket.
 */
-   copied = -ENOTCONN;
+   err = -ENOTCONN;
+   copied = 0;
break;
}
break;
}
if (!timeo) {
-   copied = -EAGAIN;
+   err = -EAGAIN;
+   copied = 0;
break;
}
}
@@ -823,7 +825,7 @@ static int llc_ui_recvmsg(struct socket *sock, struct 
msghdr *msg, size_t len,
if (rc) {
/* Exception. Bailout! */
if (!copied)
-   copied = -EFAULT;
+   err = -EFAULT;
break;
}
}
@@ -850,6 +852,8 @@ static int llc_ui_recvmsg(struct socket *sock, struct 
msghdr *msg, size_t len,
 
 out:
release_sock(sk);
+   if (err)
+   return err;
return copied;
 copy_uaddr:
if (uaddr != NULL && skb != NULL) {
-- 
2.4.10

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

[PATCH 05/11] net: irda: change chunk from int to size_t

[LABBE Corentin]

chunk cannot be negative and is use in operation/function that
wait for unsigned value. This patch set it as size_t.

Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
---
 net/irda/af_irda.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/irda/af_irda.c b/net/irda/af_irda.c
index fae6822..180cca2 100644
--- a/net/irda/af_irda.c
+++ b/net/irda/af_irda.c
@@ -1424,7 +1424,7 @@ static int irda_recvmsg_stream(struct socket *sock, 
struct msghdr *msg,
timeo = sock_rcvtimeo(sk, noblock);
 
do {
-   int chunk;
+   size_t chunk;
struct sk_buff *skb = skb_dequeue(>sk_receive_queue);
 
if (skb == NULL) {
-- 
2.4.10

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

[no subject]

[LABBE Corentin]

Hello

This patch series was begun by my finding that memcpy_[to|from]_msg have
a parameter len which is an int but used as size_t in whole functions.
Without blindly changing the parameter to size_t, I have tried to see if
anywhere in linux source code, someone give a negative argument with
the following (unfinished) coccinnelle patch.
virtual report
@@
type T;
signed T i;
@@
(
memcpy_from_msg
|
memcpy_to_msg
)
 (...,
- i)
+ (size_t)i)

With that I found many place where int variable is used to store unsigned values
and which could be set as size_t since there are used againt size_t
and/or given to functions that wait for size_t.
It permit also to found a bug in net/llc/af_llc.c where a size_t variable
stored error codes.

Regards

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

[PATCH 10/11] net: ipv4: hlen could be set as size_t

[LABBE Corentin]

The hlen member of raw_frag_vec is used in operation/function that wait
for unsigned value. So it need to be set as size_t.

Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
---
 net/ipv4/raw.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv4/raw.c b/net/ipv4/raw.c
index 561cd4b..a00aaed 100644
--- a/net/ipv4/raw.c
+++ b/net/ipv4/raw.c
@@ -86,7 +86,7 @@ struct raw_frag_vec {
struct icmphdr icmph;
char c[1];
} hdr;
-   int hlen;
+   size_t hlen;
 };
 
 static struct raw_hashinfo raw_v4_hashinfo = {
-- 
2.4.10

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

Re: [PATCH 1/4] stmmac: replace all pr_xxx by their dev_xxx counterpart

[LABBE Corentin]

On Wed, Sep 09, 2015 at 09:14:42AM -0700, Joe Perches wrote:
> On Wed, 2015-09-09 at 15:14 +0200, LABBE Corentin wrote:
> > The stmmac driver use lots of pr_xxx functions to print information.
> > This is bad since we cannot know which device logs the information.
> > (moreover if two stmmac device are present)
> []
> > So this patch replace all pr_xxx by their dev_xxx counterpart.
> 
> Using
>   netdev_(priv->dev, ...
> instead of
>   dev_(priv->device,
> 
> would be more consistent with other ethernet devices.
> 
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c 
> > b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> []
> > @@ -298,7 +298,7 @@ bool stmmac_eee_init(struct stmmac_priv *priv)
> >  */
> > spin_lock_irqsave(>lock, flags);
> > if (priv->eee_active) {
> > -   pr_debug("stmmac: disable EEE\n");
> > +   dev_dbg(priv->device, "disable EEE\n");
> 
>   netdev_dbg(priv->dev, ...)
> 
> > @@ -657,10 +657,10 @@ static int stmmac_init_ptp(struct stmmac_priv *priv)
> > priv->adv_ts = 1;
> >  
> > if (netif_msg_hw(priv) && priv->dma_cap.time_stamp)
> > -   pr_debug("IEEE 1588-2002 Time Stamp supported\n");
> > +   dev_dbg(priv->device, "IEEE 1588-2002 Time Stamp supported\n");
> 
> And these netif_msg_ could be
> 
>   if (priv->dma_cap.timestamp)
>   netif_dbg(priv, hw, priv->dev, ...);
> 
> 

Hello

My main goal is to improve logging from
[0.796804] stmmaceth 1c5.ethernet: no reset control found
[0.802635]  Ring mode enabled
[0.805713]  No HW DMA feature register supported
[0.810239]  Normal descriptors
[0.813577]  TX Checksum insertion supported
[   23.615074] eth0: device MAC address aa:65:84:d5:a3:58
[   23.704326]  RX IPC Checksum Offload disabled
[   23.704349]  No MAC Management Counters available

to that:
[0.788147] sun7i-dwmac 1c5.ethernet (unnamed net_device) 
(uninitialized): no reset control found
[0.797400] sun7i-dwmac 1c5.ethernet (unnamed net_device) 
(uninitialized): Ring mode enabled
[0.806211] sun7i-dwmac 1c5.ethernet (unnamed net_device) 
(uninitialized): No HW DMA feature register supported
[0.816658] sun7i-dwmac 1c5.ethernet (unnamed net_device) 
(uninitialized): Normal descriptors
[0.825522] sun7i-dwmac 1c5.ethernet (unnamed net_device) 
(uninitialized): TX Checksum insertion supported
[   12.971725] sun7i-dwmac 1c5.ethernet eth0: device MAC address 
3e:62:18:6f:c7:f4
[   13.056902] sun7i-dwmac 1c5.ethernet eth0: RX IPC Checksum Offload 
disabled
[   13.056929] sun7i-dwmac 1c5.ethernet eth0: No MAC Management Counters 
available

But by using the netdev_ functions the first five lines are not "pretty" with 
the "(unnamed net_device) (uninitialized)"
Could I switch back do dev_xxx since they are "early device logging" and so 
make it prettier ?

Best regards

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

[PATCH v2 4/5] stmmac: Fix simple style problem

[LABBE Corentin]

This patch fix the following warnings:
- braces {} should be used on all arms of this statement
- Prefer seq_puts to seq_printf
- No space is necessary after a cast
- Missing a blank line after declarations
- Please don't use multiple blank lines
- Comparison to NULL could be written
- networking block comments don't use an empty /* line
- Do not include the paragraph about writing to the Free Software

Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 74 +++
 1 file changed, 35 insertions(+), 39 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index c3ec169..08778b9 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -13,10 +13,6 @@
   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, write to the Free Software Foundation, Inc.,
-  51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
-
   The full GNU General Public License is included in this distribution in
   the file called "COPYING".
 
@@ -197,7 +193,7 @@ static void print_pkt(unsigned char *buf, int len)
 }
 
 /* minimum number of free TX descriptors required to wake up TX process */
-#define STMMAC_TX_THRESH(x)(x->dma_tx_size/4)
+#define STMMAC_TX_THRESH(x)(x->dma_tx_size / 4)
 
 static inline u32 stmmac_tx_avail(struct stmmac_priv *priv)
 {
@@ -207,7 +203,7 @@ static inline u32 stmmac_tx_avail(struct stmmac_priv *priv)
 /**
  * stmmac_hw_fix_mac_speed - callback for speed selection
  * @priv: driver private structure
- * Description: on some platforms (e.g. ST), some HW system configuraton
+ * Description: on some platforms (e.g. ST), some HW system configuration
  * registers have to be set according to the link speed negotiated.
  */
 static inline void stmmac_hw_fix_mac_speed(struct stmmac_priv *priv)
@@ -371,8 +367,6 @@ static void stmmac_get_tx_hwtstamp(struct stmmac_priv *priv,
shhwtstamp.hwtstamp = ns_to_ktime(ns);
/* pass tstamp to stack */
skb_tstamp_tx(skb, );
-
-   return;
 }
 
 /* stmmac_get_rx_hwtstamp - get HW RX timestamps
@@ -615,7 +609,7 @@ static int stmmac_hwtstamp_ioctl(struct net_device *dev, 
struct ifreq *ifr)
 * 2^x * y == (y << x), hence
 * 2^32 * 5000 ==> (5000 << 32)
 */
-   temp = (u64) (5000ULL << 32);
+   temp = (u64)(5000ULL << 32);
priv->default_addend = div_u64(temp, priv->clk_ptp_rate);
priv->hw->ptp->config_addend(priv->ioaddr,
 priv->default_addend);
@@ -693,7 +687,7 @@ static void stmmac_adjust_link(struct net_device *dev)
int new_state = 0;
unsigned int fc = priv->flow_ctrl, pause_time = priv->pause;
 
-   if (phydev == NULL)
+   if (!phydev)
return;
 
spin_lock_irqsave(>lock, flags);
@@ -702,7 +696,8 @@ static void stmmac_adjust_link(struct net_device *dev)
u32 ctrl = readl(priv->ioaddr + MAC_CTRL_REG);
 
/* Now we make sure that we can be in full duplex mode.
-* If not, we operate in half-duplex mode. */
+* If not, we operate in half-duplex mode.
+*/
if (phydev->duplex != priv->oldduplex) {
new_state = 1;
if (!(phydev->duplex))
@@ -728,11 +723,10 @@ static void stmmac_adjust_link(struct net_device *dev)
case 10:
if (priv->plat->has_gmac) {
ctrl |= priv->hw->link.port;
-   if (phydev->speed == SPEED_100) {
+   if (phydev->speed == SPEED_100)
ctrl |= priv->hw->link.speed;
-   } else {
+   else
ctrl &= ~(priv->hw->link.speed);
-   }
} else {
ctrl &= ~priv->hw->link.port;
}
@@ -814,6 +808,7 @@ static int stmmac_init_phy(struct net_device *dev)
char bus_id[MII_BUS_ID_SIZE];
int interface = priv->plat->interface;
int max_speed = priv->plat->max_speed;
+
priv->oldlink = 0;
priv->speed = 0;
priv->oldduplex = -1;
@@ -851,8 +846,7 @@ static int stmmac_init_phy(s

[PATCH v2 2/5] stmmac: replace hardcoded function name by __func__

[LABBE Corentin]

Some printing have the function name hardcoded.
It is better to use __func__ instead.

Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 1cfce6e..b016f04 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -831,7 +831,7 @@ static int stmmac_init_phy(struct net_device *dev)
 
snprintf(phy_id_fmt, MII_BUS_ID_SIZE + 3, PHY_ID_FMT, bus_id,
 priv->plat->phy_addr);
-   netdev_dbg(priv->dev, "stmmac_init_phy: trying to attach to 
%s\n",
+   netdev_dbg(priv->dev, "%s: trying to attach to %s\n", __func__,
   phy_id_fmt);
 
phydev = phy_connect(dev, phy_id_fmt, _adjust_link,
@@ -862,8 +862,8 @@ static int stmmac_init_phy(struct net_device *dev)
phy_disconnect(phydev);
return -ENODEV;
}
-   netdev_dbg(priv->dev, "stmmac_init_phy: %s: attached to PHY (UID 0x%x) 
Link = %d\n",
-  dev->name, phydev->phy_id, phydev->link);
+   netdev_dbg(priv->dev, "%s: %s: attached to PHY (UID 0x%x) Link = %d\n",
+  __func__, dev->name, phydev->phy_id, phydev->link);
 
priv->phydev = phydev;
 
-- 
2.4.6

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

stmmac: improve logging

[LABBE Corentin]

Hello

This patch series try to improve logging of the stmmac driver.

Changes since v1
- Use netdev_xxx instead of dev_xxx
- Use netif_xxx instead of "if (netif_msg_type) dev_xxx"

Regards

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

[PATCH v2 1/5] stmmac: replace all pr_xxx by their netdev_xxx counterpart

[LABBE Corentin]

The stmmac driver use lots of pr_xxx functions to print information.
This is bad since we cannot know which device logs the information.
(moreover if two stmmac device are present)

Furthermore, it seems that it assumes wrongly that all logs will always
be subsequent by using a dev_xxx then some indented pr_xxx like this:
kernel: sun7i-dwmac 1c5.ethernet: no reset control found
kernel:  Ring mode enabled
kernel:  No HW DMA feature register supported
kernel:  Normal descriptors
kernel:  TX Checksum insertion supported

So this patch replace all pr_xxx by their dev_xxx counterpart.

In the same time I remove some "stmmac:" print since
this will be a duplicate with that dev_xxx displays.
And this patch also change some pr_info by netdev_err when
the word ERROR is printed.

Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 216 --
 1 file changed, 122 insertions(+), 94 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 864b476..1cfce6e 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -298,7 +298,7 @@ bool stmmac_eee_init(struct stmmac_priv *priv)
 */
spin_lock_irqsave(>lock, flags);
if (priv->eee_active) {
-   pr_debug("stmmac: disable EEE\n");
+   netdev_dbg(priv->dev, "disable EEE\n");
del_timer_sync(>eee_ctrl_timer);
priv->hw->mac->set_eee_timer(priv->hw, 0,
 tx_lpi_timer);
@@ -327,7 +327,7 @@ bool stmmac_eee_init(struct stmmac_priv *priv)
ret = true;
spin_unlock_irqrestore(>lock, flags);
 
-   pr_debug("stmmac: Energy-Efficient Ethernet initialized\n");
+   netdev_dbg(priv->dev, "Energy-Efficient Ethernet 
initialized\n");
}
 out:
return ret;
@@ -448,8 +448,8 @@ static int stmmac_hwtstamp_ioctl(struct net_device *dev, 
struct ifreq *ifr)
   sizeof(struct hwtstamp_config)))
return -EFAULT;
 
-   pr_debug("%s config flags:0x%x, tx_type:0x%x, rx_filter:0x%x\n",
-__func__, config.flags, config.tx_type, config.rx_filter);
+   netdev_dbg(priv->dev, "%s config flags:0x%x, tx_type:0x%x, 
rx_filter:0x%x\n",
+  __func__, config.flags, config.tx_type, config.rx_filter);
 
/* reserved for future extensions */
if (config.flags)
@@ -657,10 +657,10 @@ static int stmmac_init_ptp(struct stmmac_priv *priv)
priv->adv_ts = 1;
 
if (netif_msg_hw(priv) && priv->dma_cap.time_stamp)
-   pr_debug("IEEE 1588-2002 Time Stamp supported\n");
+   netdev_dbg(priv->dev, "IEEE 1588-2002 Time Stamp supported\n");
 
if (netif_msg_hw(priv) && priv->adv_ts)
-   pr_debug("IEEE 1588-2008 Advanced Time Stamp supported\n");
+   netdev_dbg(priv->dev, "IEEE 1588-2008 Advanced Time Stamp 
supported\n");
 
priv->hw->ptp = _ptp;
priv->hwts_tx_en = 0;
@@ -740,8 +740,9 @@ static void stmmac_adjust_link(struct net_device *dev)
break;
default:
if (netif_msg_link(priv))
-   pr_warn("%s: Speed (%d) not 10/100\n",
-   dev->name, phydev->speed);
+   netdev_warn(priv->dev,
+   "%s: Speed (%d) not 
10/100\n",
+   dev->name, phydev->speed);
break;
}
 
@@ -788,10 +789,10 @@ static void stmmac_check_pcs_mode(struct stmmac_priv 
*priv)
(interface == PHY_INTERFACE_MODE_RGMII_ID) ||
(interface == PHY_INTERFACE_MODE_RGMII_RXID) ||
(interface == PHY_INTERFACE_MODE_RGMII_TXID)) {
-   pr_debug("STMMAC: PCS RGMII support enable\n");
+   netdev_dbg(priv->dev, "PCS RGMII support enable\n");
priv->pcs = STMMAC_PCS_RGMII;
} else if (interface == PHY_INTERFACE_MODE_SGMII) {
-   pr_debug("STMMAC: PCS SGMII support enable\n");
+   netdev_dbg(priv->dev, "PCS SGMII support enable\n");
priv->pcs = STMMAC_PCS_S

Re: [PATCH 1/4] stmmac: replace all pr_xxx by their dev_xxx counterpart

[LABBE Corentin]

On Wed, Sep 09, 2015 at 09:14:42AM -0700, Joe Perches wrote:
> On Wed, 2015-09-09 at 15:14 +0200, LABBE Corentin wrote:
> > The stmmac driver use lots of pr_xxx functions to print information.
> > This is bad since we cannot know which device logs the information.
> > (moreover if two stmmac device are present)
> []
> > So this patch replace all pr_xxx by their dev_xxx counterpart.
> 
> Using
>   netdev_(priv->dev, ...
> instead of
>   dev_(priv->device,
> 
> would be more consistent with other ethernet devices.
> 

Ok

> > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c 
> > b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> []
> > @@ -298,7 +298,7 @@ bool stmmac_eee_init(struct stmmac_priv *priv)
> >  */
> > spin_lock_irqsave(>lock, flags);
> > if (priv->eee_active) {
> > -   pr_debug("stmmac: disable EEE\n");
> > +   dev_dbg(priv->device, "disable EEE\n");
> 
>   netdev_dbg(priv->dev, ...)
> 
> > @@ -657,10 +657,10 @@ static int stmmac_init_ptp(struct stmmac_priv *priv)
> > priv->adv_ts = 1;
> >  
> > if (netif_msg_hw(priv) && priv->dma_cap.time_stamp)
> > -   pr_debug("IEEE 1588-2002 Time Stamp supported\n");
> > +   dev_dbg(priv->device, "IEEE 1588-2002 Time Stamp supported\n");
> 
> And these netif_msg_ could be
> 
>   if (priv->dma_cap.timestamp)
>   netif_dbg(priv, hw, priv->dev, ...);
> 
> 
Thanks for the tip, I will add a patch for replacing that.

Regards

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

[PATCH v2 5/5] stmmac: replace if (netif_msg_type) by their netif_xxx counterpart

[LABBE Corentin]

As sugested by Joe Perches, we could replace all
if (netif_msg_type(priv)) dev_xxx(priv->devices, ...)
by the simplier macro netif_xxx(priv, hw, priv->dev, ...)

Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 62 ++-
 1 file changed, 27 insertions(+), 35 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 08778b9..477078a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -650,11 +650,13 @@ static int stmmac_init_ptp(struct stmmac_priv *priv)
if (priv->dma_cap.atime_stamp && priv->extend_desc)
priv->adv_ts = 1;
 
-   if (netif_msg_hw(priv) && priv->dma_cap.time_stamp)
-   netdev_dbg(priv->dev, "IEEE 1588-2002 Time Stamp supported\n");
+   if (priv->dma_cap.time_stamp)
+   netif_dbg(priv, hw, priv->dev,
+ "IEEE 1588-2002 Time Stamp supported\n");
 
-   if (netif_msg_hw(priv) && priv->adv_ts)
-   netdev_dbg(priv->dev, "IEEE 1588-2008 Advanced Time Stamp 
supported\n");
+   if (priv->adv_ts)
+   netif_dbg(priv, hw, priv->dev,
+ "IEEE 1588-2008 Advanced Time Stamp supported\n");
 
priv->hw->ptp = _ptp;
priv->hwts_tx_en = 0;
@@ -733,10 +735,9 @@ static void stmmac_adjust_link(struct net_device *dev)
stmmac_hw_fix_mac_speed(priv);
break;
default:
-   if (netif_msg_link(priv))
-   netdev_warn(priv->dev,
-   "%s: Speed (%d) not 
10/100\n",
-   dev->name, phydev->speed);
+   netif_warn(priv, link, priv->dev,
+  "%s: Speed (%d) not 10/100\n",
+  dev->name, phydev->speed);
break;
}
 
@@ -1038,18 +1039,15 @@ static int init_dma_desc_rings(struct net_device *dev, 
gfp_t flags)
 
priv->dma_buf_sz = bfsize;
 
-   if (netif_msg_probe(priv))
-   netdev_dbg(priv->dev, "%s: txsize %d, rxsize %d, bfsize %d\n",
-  __func__, txsize, rxsize, bfsize);
+   netif_dbg(priv, probe, priv->dev, "%s: txsize %d, rxsize %d, bfsize 
%d\n",
+ __func__, txsize, rxsize, bfsize);
 
-   if (netif_msg_probe(priv)) {
-   netdev_dbg(priv->dev, "(%s) dma_rx_phy=0x%08x 
dma_tx_phy=0x%08x\n",
-  __func__, (u32)priv->dma_rx_phy,
-  (u32)priv->dma_tx_phy);
+   netif_dbg(priv, probe, priv->dev, "(%s) dma_rx_phy=0x%08x 
dma_tx_phy=0x%08x\n",
+ __func__, (u32)priv->dma_rx_phy, (u32)priv->dma_tx_phy);
+
+   /* RX INITIALIZATION */
+   netif_dbg(priv, probe, priv->dev, "SKB addresses:\nskb\t\tskb data\tdma 
data\n");
 
-   /* RX INITIALIZATION */
-   netdev_dbg(priv->dev, "SKB addresses:\nskb\t\tskb data\tdma 
data\n");
-   }
for (i = 0; i < rxsize; i++) {
struct dma_desc *p;
 
@@ -1062,11 +1060,9 @@ static int init_dma_desc_rings(struct net_device *dev, 
gfp_t flags)
if (ret)
goto err_init_rx_buffers;
 
-   if (netif_msg_probe(priv))
-   netdev_dbg(priv->dev, "[%p]\t[%p]\t[%x]\n",
-  priv->rx_skbuff[i],
-priv->rx_skbuff[i]->data,
-(unsigned int)priv->rx_skbuff_dma[i]);
+   netif_dbg(priv, probe, priv->dev, "[%p]\t[%p]\t[%x]\n",
+ priv->rx_skbuff[i], priv->rx_skbuff[i]->data,
+ (unsigned int)priv->rx_skbuff_dma[i]);
}
priv->cur_rx = 0;
priv->dirty_rx = (unsigned int)(i - rxsize);
@@ -1346,9 +1342,8 @@ static void stmmac_tx_clean(struct stmmac_priv *priv)
 
stmmac_get_tx_hwtstamp(priv, entry, skb);
}
-   if (netif_msg_tx_done(priv))
-   netdev_dbg(priv->dev, "%s: curr %d, dirty %d\n",
-  __func__, priv->cur_tx, priv->dirty_tx);
+   netif_dbg(priv, tx_done, priv->dev, "%s: curr %d, dirty %d\n",
+ __func__, priv->cur_tx, priv->dirty_tx)

[PATCH v2 3/5] stmmac: remove some __func__ printing

[LABBE Corentin]

Now that stmmac use netdev_xxx, some __func__ are not necessary since their
use was to clearly identify which driver was logging.

This patch remove __func__ where such printing is useless.

Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 52 +--
 1 file changed, 21 insertions(+), 31 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index b016f04..c3ec169 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -987,8 +987,7 @@ static int stmmac_init_rx_buffers(struct stmmac_priv *priv, 
struct dma_desc *p,
 
skb = __netdev_alloc_skb_ip_align(priv->dev, priv->dma_buf_sz, flags);
if (!skb) {
-   netdev_err(priv->dev,
-  "%s: Rx init fails; skb is NULL\n", __func__);
+   netdev_err(priv->dev, "Rx init fails; skb is NULL\n");
return -ENOMEM;
}
priv->rx_skbuff[i] = skb;
@@ -996,7 +995,7 @@ static int stmmac_init_rx_buffers(struct stmmac_priv *priv, 
struct dma_desc *p,
priv->dma_buf_sz,
DMA_FROM_DEVICE);
if (dma_mapping_error(priv->device, priv->rx_skbuff_dma[i])) {
-   netdev_err(priv->dev, "%s: DMA mapping error\n", __func__);
+   netdev_err(priv->dev, "DMA mapping error\n");
dev_kfree_skb_any(skb);
return -EINVAL;
}
@@ -1710,8 +1709,7 @@ static int stmmac_hw_setup(struct net_device *dev, bool 
init_ptp)
/* DMA initialization and SW reset */
ret = stmmac_init_dma_engine(priv);
if (ret < 0) {
-   netdev_err(priv->dev, "%s: DMA engine initialization failed\n",
-  __func__);
+   netdev_err(priv->dev, "DMA engine initialization failed\n");
return ret;
}
 
@@ -1743,15 +1741,13 @@ static int stmmac_hw_setup(struct net_device *dev, bool 
init_ptp)
if (init_ptp) {
ret = stmmac_init_ptp(priv);
if (ret && ret != -EOPNOTSUPP)
-   netdev_warn(priv->dev, "%s: failed PTP 
initialisation\n",
-   __func__);
+   netdev_warn(priv->dev, "failed PTP initialisation\n");
}
 
 #ifdef CONFIG_DEBUG_FS
ret = stmmac_init_fs(dev);
if (ret < 0)
-   netdev_warn(priv->dev, "%s: failed debugFS registration\n",
-   __func__);
+   netdev_warn(priv->dev, "failed debugFS registration\n");
 #endif
/* Start the ball rolling... */
netdev_dbg(priv->dev, "%s: DMA RX/TX processes started...\n",
@@ -1798,8 +1794,7 @@ static int stmmac_open(struct net_device *dev)
ret = stmmac_init_phy(dev);
if (ret) {
netdev_err(priv->dev,
-  "%s: Cannot attach to PHY (error: %d)\n",
-  __func__, ret);
+  "Cannot attach to PHY (error: %d)\n", ret);
return ret;
}
}
@@ -1815,21 +1810,19 @@ static int stmmac_open(struct net_device *dev)
 
ret = alloc_dma_desc_resources(priv);
if (ret < 0) {
-   netdev_err(priv->dev, "%s: DMA descriptors allocation failed\n"
-  __func__);
+   netdev_err(priv->dev, "DMA descriptors allocation failed\n");
goto dma_desc_error;
}
 
ret = init_dma_desc_rings(dev, GFP_KERNEL);
if (ret < 0) {
-   netdev_err(priv->dev, "%s: DMA descriptors initialization 
failed\n",
-  __func__);
+   netdev_err(priv->dev, "DMA descriptors initialization 
failed\n");
goto init_error;
}
 
ret = stmmac_hw_setup(dev, true);
if (ret < 0) {
-   netdev_err(priv->dev, "%s: Hw setup failed\n", __func__);
+   netdev_err(priv->dev, "Hw setup failed\n");
goto init_error;
}
 
@@ -1843,8 +1836,8 @@ static int stmmac_open(struct net_device *dev)
  IRQF_SHARED, dev->name, dev);
if (unlikely(ret < 0)) {
netdev_err(priv->dev,
-  "%s: ERROR: allocating the IRQ %d (error: %d)\n",
-  __func__, dev->irq, ret);
+  "ERROR: allocating the IRQ %d

[PATCH 4/4] stmmac: Fix simple style problem

[LABBE Corentin]

This patch fix the following warnings:
- braces {} should be used on all arms of this statement
- Prefer seq_puts to seq_printf
- No space is necessary after a cast
- Missing a blank line after declarations
- Please don't use multiple blank lines
- Comparison to NULL could be written
- networking block comments don't use an empty /* line
- Do not include the paragraph about writing to the Free Software

Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 74 +++
 1 file changed, 35 insertions(+), 39 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 5335bad..3ba95a7 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -13,10 +13,6 @@
   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, write to the Free Software Foundation, Inc.,
-  51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
-
   The full GNU General Public License is included in this distribution in
   the file called "COPYING".
 
@@ -197,7 +193,7 @@ static void print_pkt(unsigned char *buf, int len)
 }
 
 /* minimum number of free TX descriptors required to wake up TX process */
-#define STMMAC_TX_THRESH(x)(x->dma_tx_size/4)
+#define STMMAC_TX_THRESH(x)(x->dma_tx_size / 4)
 
 static inline u32 stmmac_tx_avail(struct stmmac_priv *priv)
 {
@@ -207,7 +203,7 @@ static inline u32 stmmac_tx_avail(struct stmmac_priv *priv)
 /**
  * stmmac_hw_fix_mac_speed - callback for speed selection
  * @priv: driver private structure
- * Description: on some platforms (e.g. ST), some HW system configuraton
+ * Description: on some platforms (e.g. ST), some HW system configuration
  * registers have to be set according to the link speed negotiated.
  */
 static inline void stmmac_hw_fix_mac_speed(struct stmmac_priv *priv)
@@ -371,8 +367,6 @@ static void stmmac_get_tx_hwtstamp(struct stmmac_priv *priv,
shhwtstamp.hwtstamp = ns_to_ktime(ns);
/* pass tstamp to stack */
skb_tstamp_tx(skb, );
-
-   return;
 }
 
 /* stmmac_get_rx_hwtstamp - get HW RX timestamps
@@ -615,7 +609,7 @@ static int stmmac_hwtstamp_ioctl(struct net_device *dev, 
struct ifreq *ifr)
 * 2^x * y == (y << x), hence
 * 2^32 * 5000 ==> (5000 << 32)
 */
-   temp = (u64) (5000ULL << 32);
+   temp = (u64)(5000ULL << 32);
priv->default_addend = div_u64(temp, priv->clk_ptp_rate);
priv->hw->ptp->config_addend(priv->ioaddr,
 priv->default_addend);
@@ -693,7 +687,7 @@ static void stmmac_adjust_link(struct net_device *dev)
int new_state = 0;
unsigned int fc = priv->flow_ctrl, pause_time = priv->pause;
 
-   if (phydev == NULL)
+   if (!phydev)
return;
 
spin_lock_irqsave(>lock, flags);
@@ -702,7 +696,8 @@ static void stmmac_adjust_link(struct net_device *dev)
u32 ctrl = readl(priv->ioaddr + MAC_CTRL_REG);
 
/* Now we make sure that we can be in full duplex mode.
-* If not, we operate in half-duplex mode. */
+* If not, we operate in half-duplex mode.
+*/
if (phydev->duplex != priv->oldduplex) {
new_state = 1;
if (!(phydev->duplex))
@@ -728,11 +723,10 @@ static void stmmac_adjust_link(struct net_device *dev)
case 10:
if (priv->plat->has_gmac) {
ctrl |= priv->hw->link.port;
-   if (phydev->speed == SPEED_100) {
+   if (phydev->speed == SPEED_100)
ctrl |= priv->hw->link.speed;
-   } else {
+   else
ctrl &= ~(priv->hw->link.speed);
-   }
} else {
ctrl &= ~priv->hw->link.port;
}
@@ -814,6 +808,7 @@ static int stmmac_init_phy(struct net_device *dev)
char bus_id[MII_BUS_ID_SIZE];
int interface = priv->plat->interface;
int max_speed = priv->plat->max_speed;
+
priv->oldlink = 0;
priv->speed = 0;
priv->oldduplex = -1;
@@ -851,8 +846,7 @@ static int stmmac_init_phy(s

[PATCH 3/4] stmmac: remove some __func__ printing

[LABBE Corentin]

Now that stmmac use dev_xxx, some __func__ are not necessary since their
use was to clearly identify which driver was logging.

This patch remove __func__ where such printing is useless.

Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 57 +--
 1 file changed, 21 insertions(+), 36 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index c1dc41b..5335bad 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -988,7 +988,7 @@ static int stmmac_init_rx_buffers(struct stmmac_priv *priv, 
struct dma_desc *p,
skb = __netdev_alloc_skb_ip_align(priv->dev, priv->dma_buf_sz, flags);
if (!skb) {
dev_err(priv->device,
-   "%s: Rx init fails; skb is NULL\n", __func__);
+   "Rx init fails; skb is NULL\n");
return -ENOMEM;
}
priv->rx_skbuff[i] = skb;
@@ -996,7 +996,7 @@ static int stmmac_init_rx_buffers(struct stmmac_priv *priv, 
struct dma_desc *p,
priv->dma_buf_sz,
DMA_FROM_DEVICE);
if (dma_mapping_error(priv->device, priv->rx_skbuff_dma[i])) {
-   dev_err(priv->device, "%s: DMA mapping error\n", __func__);
+   dev_err(priv->device, "DMA mapping error\n");
dev_kfree_skb_any(skb);
return -EINVAL;
}
@@ -1709,8 +1709,7 @@ static int stmmac_hw_setup(struct net_device *dev, bool 
init_ptp)
/* DMA initialization and SW reset */
ret = stmmac_init_dma_engine(priv);
if (ret < 0) {
-   dev_err(priv->device, "%s: DMA engine initialization failed\n",
-   __func__);
+   dev_err(priv->device, "DMA engine initialization failed\n");
return ret;
}
 
@@ -1742,15 +1741,13 @@ static int stmmac_hw_setup(struct net_device *dev, bool 
init_ptp)
if (init_ptp) {
ret = stmmac_init_ptp(priv);
if (ret && ret != -EOPNOTSUPP)
-   dev_warn(priv->device, "%s: failed PTP 
initialisation\n",
-__func__);
+   dev_warn(priv->device, "failed PTP initialisation\n");
}
 
 #ifdef CONFIG_DEBUG_FS
ret = stmmac_init_fs(dev);
if (ret < 0)
-   dev_warn(priv->device, "%s: failed debugFS registration\n",
-__func__);
+   dev_warn(priv->device, "failed debugFS registration\n");
 #endif
/* Start the ball rolling... */
dev_dbg(priv->device, "%s: DMA RX/TX processes started...\n",
@@ -1797,8 +1794,7 @@ static int stmmac_open(struct net_device *dev)
ret = stmmac_init_phy(dev);
if (ret) {
dev_err(priv->device,
-   "%s: Cannot attach to PHY (error: %d)\n",
-  __func__, ret);
+   "Cannot attach to PHY (error: %d)\n", ret);
return ret;
}
}
@@ -1814,22 +1810,19 @@ static int stmmac_open(struct net_device *dev)
 
ret = alloc_dma_desc_resources(priv);
if (ret < 0) {
-   dev_err(priv->device, "%s: DMA descriptors allocation failed\n",
-   __func__);
+   dev_err(priv->device, "DMA descriptors allocation failed\n");
goto dma_desc_error;
}
 
ret = init_dma_desc_rings(dev, GFP_KERNEL);
if (ret < 0) {
-   dev_err(priv->device,
-   "%s: DMA descriptors initialization failed\n",
-   __func__);
+   dev_err(priv->device, "DMA descriptors initialization 
failed\n");
goto init_error;
}
 
ret = stmmac_hw_setup(dev, true);
if (ret < 0) {
-   dev_err(priv->device, "%s: Hw setup failed\n", __func__);
+   dev_err(priv->device, "Hw setup failed\n");
goto init_error;
}
 
@@ -1842,9 +1835,8 @@ static int stmmac_open(struct net_device *dev)
ret = request_irq(dev->irq, stmmac_interrupt,
  IRQF_SHARED, dev->name, dev);
if (unlikely(ret < 0)) {
-   dev_err(priv->device,
-   "%s: ERROR: allocating the IRQ %d (error: %d)\n",
-  __func__, dev->irq, ret);
+   dev_err(priv->devic

[PATCH 2/4] stmmac: replace hardcoded function name by __func__

[LABBE Corentin]

Some printing have the function name hardcoded.
It is better to use __func__ instead.

Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index cb9efb3..c1dc41b 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -831,7 +831,7 @@ static int stmmac_init_phy(struct net_device *dev)
 
snprintf(phy_id_fmt, MII_BUS_ID_SIZE + 3, PHY_ID_FMT, bus_id,
 priv->plat->phy_addr);
-   dev_dbg(priv->device, "stmmac_init_phy: trying to attach to 
%s\n",
+   dev_dbg(priv->device, "%s: trying to attach to %s\n", __func__,
phy_id_fmt);
 
phydev = phy_connect(dev, phy_id_fmt, _adjust_link,
@@ -862,8 +862,8 @@ static int stmmac_init_phy(struct net_device *dev)
phy_disconnect(phydev);
return -ENODEV;
}
-   dev_dbg(priv->device, "stmmac_init_phy: %s: attached to PHY (UID 0x%x) 
Link = %d\n",
-   dev->name, phydev->phy_id, phydev->link);
+   dev_dbg(priv->device, "%s: %s: attached to PHY (UID 0x%x) Link = %d\n",
+   __func__, dev->name, phydev->phy_id, phydev->link);
 
priv->phydev = phydev;
 
-- 
2.4.6

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

[PATCH 1/4] stmmac: replace all pr_xxx by their dev_xxx counterpart

[LABBE Corentin]

The stmmac driver use lots of pr_xxx functions to print information.
This is bad since we cannot know which device logs the information.
(moreover if two stmmac device are present)

Furthermore, it seems that it assumes wrongly that all logs will always
be subsequent by using a dev_xxx then some indented pr_xxx like this:
kernel: sun7i-dwmac 1c5.ethernet: no reset control found
kernel:  Ring mode enabled
kernel:  No HW DMA feature register supported
kernel:  Normal descriptors
kernel:  TX Checksum insertion supported

So this patch replace all pr_xxx by their dev_xxx counterpart.

In the same time I remove some "stmmac:" print since
this will be a duplicate with that dev_xxx displays.
And this patch also change some pr_info by dev_err when
the word ERROR is printed.

Signed-off-by: LABBE Corentin <clabbe.montj...@gmail.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 197 --
 1 file changed, 112 insertions(+), 85 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 864b476..cb9efb3 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -298,7 +298,7 @@ bool stmmac_eee_init(struct stmmac_priv *priv)
 */
spin_lock_irqsave(>lock, flags);
if (priv->eee_active) {
-   pr_debug("stmmac: disable EEE\n");
+   dev_dbg(priv->device, "disable EEE\n");
del_timer_sync(>eee_ctrl_timer);
priv->hw->mac->set_eee_timer(priv->hw, 0,
 tx_lpi_timer);
@@ -327,7 +327,7 @@ bool stmmac_eee_init(struct stmmac_priv *priv)
ret = true;
spin_unlock_irqrestore(>lock, flags);
 
-   pr_debug("stmmac: Energy-Efficient Ethernet initialized\n");
+   dev_dbg(priv->device, "Energy-Efficient Ethernet 
initialized\n");
}
 out:
return ret;
@@ -448,8 +448,8 @@ static int stmmac_hwtstamp_ioctl(struct net_device *dev, 
struct ifreq *ifr)
   sizeof(struct hwtstamp_config)))
return -EFAULT;
 
-   pr_debug("%s config flags:0x%x, tx_type:0x%x, rx_filter:0x%x\n",
-__func__, config.flags, config.tx_type, config.rx_filter);
+   dev_dbg(priv->device, "%s config flags:0x%x, tx_type:0x%x, 
rx_filter:0x%x\n",
+   __func__, config.flags, config.tx_type, config.rx_filter);
 
/* reserved for future extensions */
if (config.flags)
@@ -657,10 +657,10 @@ static int stmmac_init_ptp(struct stmmac_priv *priv)
priv->adv_ts = 1;
 
if (netif_msg_hw(priv) && priv->dma_cap.time_stamp)
-   pr_debug("IEEE 1588-2002 Time Stamp supported\n");
+   dev_dbg(priv->device, "IEEE 1588-2002 Time Stamp supported\n");
 
if (netif_msg_hw(priv) && priv->adv_ts)
-   pr_debug("IEEE 1588-2008 Advanced Time Stamp supported\n");
+   dev_dbg(priv->device, "IEEE 1588-2008 Advanced Time Stamp 
supported\n");
 
priv->hw->ptp = _ptp;
priv->hwts_tx_en = 0;
@@ -740,8 +740,9 @@ static void stmmac_adjust_link(struct net_device *dev)
break;
default:
if (netif_msg_link(priv))
-   pr_warn("%s: Speed (%d) not 10/100\n",
-   dev->name, phydev->speed);
+   dev_warn(priv->device,
+"%s: Speed (%d) not 10/100\n",
+dev->name, phydev->speed);
break;
}
 
@@ -788,10 +789,10 @@ static void stmmac_check_pcs_mode(struct stmmac_priv 
*priv)
(interface == PHY_INTERFACE_MODE_RGMII_ID) ||
(interface == PHY_INTERFACE_MODE_RGMII_RXID) ||
(interface == PHY_INTERFACE_MODE_RGMII_TXID)) {
-   pr_debug("STMMAC: PCS RGMII support enable\n");
+   dev_dbg(priv->device, "PCS RGMII support enable\n");
priv->pcs = STMMAC_PCS_RGMII;
} else if (interface == PHY_INTERFACE_MODE_SGMII) {
-   pr_debug("STMMAC: PCS SGMII support enable\n");
+   dev_dbg(priv->device, "PCS SGMII support enable\n");
priv->pcs = STMMAC_PCS_SGMII;