RE: [PATCH 0/3] net: fec: Reset ethernet PHY whenever the enet_out clock

[Duan Andy]

From: Lothar Waßmann  Sent: Tuesday, December 01, 
2015 3:25 PM
> To: Duan Fugang-B38611
> Cc: Andrew Lunn; David S. Miller; Estevam Fabio-R49496; Greg Ungerer;
> Kevin Hao; Lucas Stach; Philippe Reynes; Richard Cochran; Russell King;
> Sascha Hauer; Stefan Agner; linux-kernel@vger.kernel.org;
> net...@vger.kernel.org; Jeff Kirsher; Uwe Kleine-K?nig
> Subject: Re: [PATCH 0/3] net: fec: Reset ethernet PHY whenever the
> enet_out clock
> 
> Hi,
> 
> > From: Lothar Waßmann  Sent: Monday, November
> > 30, 2015 7:33 PM
> > > To: Andrew Lunn; David S. Miller; Estevam Fabio-R49496; Greg
> > > Ungerer; Kevin Hao; Lothar Waßmann; Lucas Stach; Duan Fugang-B38611;
> > > Philippe Reynes; Richard Cochran; Russell King; Sascha Hauer; Stefan
> > > Agner; linux- ker...@vger.kernel.org; net...@vger.kernel.org; Jeff
> > > Kirsher; Uwe Kleine- König
> > > Subject: [PATCH 0/3] net: fec: Reset ethernet PHY whenever the
> > > enet_out clock
> > >
> > > This patchset fixes a regression introduced by commit 8fff755e9f8d
> ("net:
> > > fec: Ensure clocks are enabled while using mdio bus") for ethernet
> > > PHYs that are using ENET_OUT as reference clock (on i.MX6 or i.MX28)
> > >
> > Do you mean commit 8fff755e9f8d cause your problem ?  This commit just
> manage ipg clock in runtime because mdio bus can access external phy
> switch no matter netdev status.
> >
> No. Actually I meant commit e8fcfcd5684a ("net: fec: optimize the clock
> management to save power") which started to disable the clocks when not
> in use.
> 
Understand. Yes, disable enet_out clock for power saving is necessary.

> Sorry for the confusion,
> Lothar Waßmann

RE: [PATCH 2/3] net: fec: convert to using gpiod framework

[Duan Andy]

From: Lothar Waßmann  Sent: Monday, November 30, 2015 
7:33 PM
> To: Andrew Lunn; David S. Miller; Estevam Fabio-R49496; Greg Ungerer;
> Kevin Hao; Lothar Waßmann; Lucas Stach; Duan Fugang-B38611; Philippe
> Reynes; Richard Cochran; Russell King; Sascha Hauer; Stefan Agner; linux-
> ker...@vger.kernel.org; net...@vger.kernel.org; Jeff Kirsher; Uwe Kleine-
> König
> Subject: [PATCH 2/3] net: fec: convert to using gpiod framework
> 
> Use gpiod_get_optional() instead of checking for a valid GPIO number and
> calling devm_gpio_request_one() conditionally.
> 
> Signed-off-by: Lothar Waßmann 
> ---
>  drivers/net/ethernet/freescale/fec_main.c | 17 +++--
>  1 file changed, 7 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/fec_main.c
> b/drivers/net/ethernet/freescale/fec_main.c
> index e17d74f..1a983fc 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -3233,7 +3233,7 @@ static int fec_enet_init(struct net_device *ndev)
> #ifdef CONFIG_OF  static void fec_reset_phy(struct platform_device *pdev)
> {
> - int err, phy_reset;
> + struct gpio_desc *phy_reset;
>   int msec = 1;
>   struct device_node *np = pdev->dev.of_node;
> 
> @@ -3245,18 +3245,15 @@ static void fec_reset_phy(struct platform_device
> *pdev)
>   if (msec > 1000)
>   msec = 1;
> 
> - phy_reset = of_get_named_gpio(np, "phy-reset-gpios", 0);
> - if (!gpio_is_valid(phy_reset))
> - return;
> -
> - err = devm_gpio_request_one(>dev, phy_reset,
> - GPIOF_OUT_INIT_LOW, "phy-reset");
> - if (err) {
> - dev_err(>dev, "failed to get phy-reset-gpios: %d\n",
> err);
> + phy_reset = devm_gpiod_get_optional(>dev, "phy-reset",
> + GPIOD_OUT_LOW);
> + if (IS_ERR(phy_reset)) {
> + dev_err(>dev, "failed to get phy-reset-gpios: %ld\n",
> + PTR_ERR(phy_reset));
>   return;
>   }
>   msleep(msec);
> - gpio_set_value_cansleep(phy_reset, 1);
> + gpiod_set_value_cansleep(phy_reset, 1);

This API will judge the GPIO active polarity, there many imx boards in dts 
files don't care the polarity.
So pls drop the patch.

Or use gpiod_set_raw_value_cansleep() instead of gpiod_set_value_cansleep().

>  }
>  #else /* CONFIG_OF */
>  static void fec_reset_phy(struct platform_device *pdev)
> --
> 2.1.4

RE: [PATCH 3/3] net: fec: Reset ethernet PHY whenever the enet_out clock is being enabled

[Duan Andy]

From: Lothar Waßmann  Sent: Monday, November 30, 2015 
7:33 PM
> To: Andrew Lunn; David S. Miller; Estevam Fabio-R49496; Greg Ungerer;
> Kevin Hao; Lothar Waßmann; Lucas Stach; Duan Fugang-B38611; Philippe
> Reynes; Richard Cochran; Russell King; Sascha Hauer; Stefan Agner; linux-
> ker...@vger.kernel.org; net...@vger.kernel.org; Jeff Kirsher; Uwe Kleine-
> König
> Subject: [PATCH 3/3] net: fec: Reset ethernet PHY whenever the enet_out
> clock is being enabled
> 
> If a PHY uses ENET_OUT as reference clock, it may need a RESET to get
> functional after the clock had been disabled.
> 
> Failure to do this results in the link state constantly toggling between
> up and down:
> fec 02188000.ethernet eth0: Link is Up - 100Mbps/Full - flow control
> rx/tx fec 02188000.ethernet eth0: Link is Down fec 02188000.ethernet eth0:
> Link is Up - 100Mbps/Full - flow control rx/tx fec 02188000.ethernet eth0:
> Link is Down [...]
> 
> Signed-off-by: Lothar Waßmann 
> ---
>  drivers/net/ethernet/freescale/fec.h  |  1 +
>  drivers/net/ethernet/freescale/fec_main.c | 46 -
> --
>  2 files changed, 37 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/fec.h
> b/drivers/net/ethernet/freescale/fec.h
> index 99d33e2..8ab4f7f 100644
> --- a/drivers/net/ethernet/freescale/fec.h
> +++ b/drivers/net/ethernet/freescale/fec.h
> @@ -519,6 +519,7 @@ struct fec_enet_private {
>   int pause_flag;
>   int wol_flag;
>   u32 quirks;
> + struct gpio_desc *phy_reset;
> 
>   struct  napi_struct napi;
>   int csum_flags;
> diff --git a/drivers/net/ethernet/freescale/fec_main.c
> b/drivers/net/ethernet/freescale/fec_main.c
> index 1a983fc..7ba2bbb 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -66,6 +66,7 @@
> 
>  static void set_multicast_list(struct net_device *ndev);  static void
> fec_enet_itr_coal_init(struct net_device *ndev);
> +static void fec_reset_phy(struct platform_device *pdev);
> 
>  #define DRIVER_NAME  "fec"
> 
> @@ -1861,6 +1862,8 @@ static int fec_enet_clk_enable(struct net_device
> *ndev, bool enable)
>   ret = clk_prepare_enable(fep->clk_enet_out);
>   if (ret)
>   goto failed_clk_enet_out;
> +
> + fec_reset_phy(fep->pdev);
>   }
>   if (fep->clk_ptp) {
>   mutex_lock(>ptp_clk_mutex);
> @@ -3231,13 +3234,32 @@ static int fec_enet_init(struct net_device
> *ndev)  }
> 
>  #ifdef CONFIG_OF
> -static void fec_reset_phy(struct platform_device *pdev)
> +static struct gpio_desc *fec_get_reset_gpio(struct platform_device
> +*pdev)
>  {
>   struct gpio_desc *phy_reset;
> - int msec = 1;
>   struct device_node *np = pdev->dev.of_node;
> 
>   if (!np)
> + return ERR_PTR(-ENODEV);

No need to check device node.

> +
> + phy_reset = devm_gpiod_get_optional(>dev, "phy-reset",
> + GPIOD_OUT_LOW);
> + if (IS_ERR(phy_reset))
> + dev_err(>dev, "failed to get phy-reset-gpios: %ld\n",
> + PTR_ERR(phy_reset));
> + return phy_reset;
> +}
> +
> +static void fec_reset_phy(struct platform_device *pdev) {
> + struct device_node *np = pdev->dev.of_node;
> + struct net_device *ndev = platform_get_drvdata(pdev);
> + struct fec_enet_private *fep = netdev_priv(ndev);
> + int msec = 1;
> +
> + if (!fep->phy_reset)
> + return;
> + if (!np)
>   return;

No need to check device node.

> 
>   of_property_read_u32(np, "phy-reset-duration", ); @@ -3245,17
> +3267,16 @@ static void fec_reset_phy(struct platform_device *pdev)
>   if (msec > 1000)
>   msec = 1;
> 
> - phy_reset = devm_gpiod_get_optional(>dev, "phy-reset",
> - GPIOD_OUT_LOW);
> - if (IS_ERR(phy_reset)) {
> - dev_err(>dev, "failed to get phy-reset-gpios: %ld\n",
> - PTR_ERR(phy_reset));
> - return;
> - }
> + gpiod_set_value_cansleep(fep->phy_reset, 0);
>   msleep(msec);
> - gpiod_set_value_cansleep(phy_reset, 1);
> + gpiod_set_value_cansleep(fep->phy_reset, 1);
>  }
>  #else /* CONFIG_OF */
> +static void fec_get_reset_gpio(struct platform_device *pdev) {
> + return -EINVAL;
> +}
> +
>  static void fec_reset_phy(struct platform_device *pdev)  {
>   /*
> @@ -3309,8 +3330,12 @@ fec_probe(struct platform_device *pdev)
>   struct device_node *np = pdev->dev.of_node, *phy_node;
>   int num_tx_qs;
>   int num_rx_qs;
> + struct gpio_desc *phy_reset;
> 
>   fec_enet_get_queue_num(pdev, _tx_qs, _rx_qs);
> + phy_reset = fec_get_reset_gpio(pdev);
> + if (IS_ERR(phy_reset) && PTR_ERR(phy_reset) == -EPROBE_DEFER)
> + return -EPROBE_DEFER;
> 
>   /* Init network device */
>   ndev = 

RE: [PATCH 1/3] net: fec: Remove redundant checks for NULL clk pointer

[Duan Andy]

From: Lothar Waßmann  Sent: Monday, November 30, 2015 
7:33 PM
> To: Andrew Lunn; David S. Miller; Estevam Fabio-R49496; Greg Ungerer;
> Kevin Hao; Lothar Waßmann; Lucas Stach; Duan Fugang-B38611; Philippe
> Reynes; Richard Cochran; Russell King; Sascha Hauer; Stefan Agner; linux-
> ker...@vger.kernel.org; net...@vger.kernel.org; Jeff Kirsher; Uwe Kleine-
> König
> Subject: [PATCH 1/3] net: fec: Remove redundant checks for NULL clk
> pointer
> 
> NULL is a valid argument to clk_enable()/clk_disable(). Remove redundant
> checks before calling those functions.
> 
> Signed-off-by: Lothar Waßmann 
> ---
>  drivers/net/ethernet/freescale/fec_main.c | 23 +--
>  1 file changed, 9 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/fec_main.c
> b/drivers/net/ethernet/freescale/fec_main.c
> index d2328fc..e17d74f 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -1873,35 +1873,30 @@ static int fec_enet_clk_enable(struct net_device
> *ndev, bool enable)
>   }
>   mutex_unlock(>ptp_clk_mutex);
>   }
> - if (fep->clk_ref) {
> - ret = clk_prepare_enable(fep->clk_ref);
> - if (ret)
> - goto failed_clk_ref;
> - }
> +
> + ret = clk_prepare_enable(fep->clk_ref);
> + if (ret)
> + goto failed_clk_ref;

If you want to clean up the code, pls also remove "fep->clk_enet_out" check in 
this brace.


>   } else {
>   clk_disable_unprepare(fep->clk_ahb);
> - if (fep->clk_enet_out)
> - clk_disable_unprepare(fep->clk_enet_out);
> + clk_disable_unprepare(fep->clk_enet_out);
>   if (fep->clk_ptp) {
>   mutex_lock(>ptp_clk_mutex);
>   clk_disable_unprepare(fep->clk_ptp);
>   fep->ptp_clk_on = false;
>   mutex_unlock(>ptp_clk_mutex);
>   }
> - if (fep->clk_ref)
> - clk_disable_unprepare(fep->clk_ref);
> + clk_disable_unprepare(fep->clk_ref);
>   }
> 
>   return 0;
> 
>  failed_clk_ref:
> - if (fep->clk_ref)
> - clk_disable_unprepare(fep->clk_ref);
> + clk_disable_unprepare(fep->clk_ref);
>  failed_clk_ptp:
> - if (fep->clk_enet_out)
> - clk_disable_unprepare(fep->clk_enet_out);
> + clk_disable_unprepare(fep->clk_enet_out);
>  failed_clk_enet_out:
> - clk_disable_unprepare(fep->clk_ahb);
> + clk_disable_unprepare(fep->clk_ahb);
> 
>   return ret;
>  }
> --
> 2.1.4

RE: [PATCH 0/3] net: fec: Reset ethernet PHY whenever the enet_out clock

[Duan Andy]

From: Lothar Waßmann  Sent: Monday, November 30, 2015 
7:33 PM
> To: Andrew Lunn; David S. Miller; Estevam Fabio-R49496; Greg Ungerer;
> Kevin Hao; Lothar Waßmann; Lucas Stach; Duan Fugang-B38611; Philippe
> Reynes; Richard Cochran; Russell King; Sascha Hauer; Stefan Agner; linux-
> ker...@vger.kernel.org; net...@vger.kernel.org; Jeff Kirsher; Uwe Kleine-
> König
> Subject: [PATCH 0/3] net: fec: Reset ethernet PHY whenever the enet_out
> clock
> 
> This patchset fixes a regression introduced by commit 8fff755e9f8d ("net:
> fec: Ensure clocks are enabled while using mdio bus") for ethernet PHYs
> that are using ENET_OUT as reference clock (on i.MX6 or i.MX28)
> 
Do you mean commit 8fff755e9f8d cause your problem ?  This commit just manage 
ipg clock in runtime because mdio bus can access external phy switch no matter 
netdev status.

I don't think the commit cause phy link up/down issue.  Phy link up/down is due 
to phy is not ready after power/clock on, it need to do reset.


> The first patch is a cleanup patch that removes redundant NULL checks.
> The second patch converts the driver to use the 'gpiod' framework.
> The third patch makes sure, fec_reset_phy() is called whenever the
> enet_out clock has been (re-)enabled to get the PHY into a
> consistent state.

RE: [PATCH 0/3] net: fec: Reset ethernet PHY whenever the enet_out clock

[Duan Andy]

From: Lothar Waßmann  Sent: Monday, November 30, 2015 
7:33 PM
> To: Andrew Lunn; David S. Miller; Estevam Fabio-R49496; Greg Ungerer;
> Kevin Hao; Lothar Waßmann; Lucas Stach; Duan Fugang-B38611; Philippe
> Reynes; Richard Cochran; Russell King; Sascha Hauer; Stefan Agner; linux-
> ker...@vger.kernel.org; net...@vger.kernel.org; Jeff Kirsher; Uwe Kleine-
> König
> Subject: [PATCH 0/3] net: fec: Reset ethernet PHY whenever the enet_out
> clock
> 
> This patchset fixes a regression introduced by commit 8fff755e9f8d ("net:
> fec: Ensure clocks are enabled while using mdio bus") for ethernet PHYs
> that are using ENET_OUT as reference clock (on i.MX6 or i.MX28)
> 
Do you mean commit 8fff755e9f8d cause your problem ?  This commit just manage 
ipg clock in runtime because mdio bus can access external phy switch no matter 
netdev status.

I don't think the commit cause phy link up/down issue.  Phy link up/down is due 
to phy is not ready after power/clock on, it need to do reset.


> The first patch is a cleanup patch that removes redundant NULL checks.
> The second patch converts the driver to use the 'gpiod' framework.
> The third patch makes sure, fec_reset_phy() is called whenever the
> enet_out clock has been (re-)enabled to get the PHY into a
> consistent state.

RE: [PATCH 1/3] net: fec: Remove redundant checks for NULL clk pointer

[Duan Andy]

From: Lothar Waßmann  Sent: Monday, November 30, 2015 
7:33 PM
> To: Andrew Lunn; David S. Miller; Estevam Fabio-R49496; Greg Ungerer;
> Kevin Hao; Lothar Waßmann; Lucas Stach; Duan Fugang-B38611; Philippe
> Reynes; Richard Cochran; Russell King; Sascha Hauer; Stefan Agner; linux-
> ker...@vger.kernel.org; net...@vger.kernel.org; Jeff Kirsher; Uwe Kleine-
> König
> Subject: [PATCH 1/3] net: fec: Remove redundant checks for NULL clk
> pointer
> 
> NULL is a valid argument to clk_enable()/clk_disable(). Remove redundant
> checks before calling those functions.
> 
> Signed-off-by: Lothar Waßmann 
> ---
>  drivers/net/ethernet/freescale/fec_main.c | 23 +--
>  1 file changed, 9 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/fec_main.c
> b/drivers/net/ethernet/freescale/fec_main.c
> index d2328fc..e17d74f 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -1873,35 +1873,30 @@ static int fec_enet_clk_enable(struct net_device
> *ndev, bool enable)
>   }
>   mutex_unlock(>ptp_clk_mutex);
>   }
> - if (fep->clk_ref) {
> - ret = clk_prepare_enable(fep->clk_ref);
> - if (ret)
> - goto failed_clk_ref;
> - }
> +
> + ret = clk_prepare_enable(fep->clk_ref);
> + if (ret)
> + goto failed_clk_ref;

If you want to clean up the code, pls also remove "fep->clk_enet_out" check in 
this brace.


>   } else {
>   clk_disable_unprepare(fep->clk_ahb);
> - if (fep->clk_enet_out)
> - clk_disable_unprepare(fep->clk_enet_out);
> + clk_disable_unprepare(fep->clk_enet_out);
>   if (fep->clk_ptp) {
>   mutex_lock(>ptp_clk_mutex);
>   clk_disable_unprepare(fep->clk_ptp);
>   fep->ptp_clk_on = false;
>   mutex_unlock(>ptp_clk_mutex);
>   }
> - if (fep->clk_ref)
> - clk_disable_unprepare(fep->clk_ref);
> + clk_disable_unprepare(fep->clk_ref);
>   }
> 
>   return 0;
> 
>  failed_clk_ref:
> - if (fep->clk_ref)
> - clk_disable_unprepare(fep->clk_ref);
> + clk_disable_unprepare(fep->clk_ref);
>  failed_clk_ptp:
> - if (fep->clk_enet_out)
> - clk_disable_unprepare(fep->clk_enet_out);
> + clk_disable_unprepare(fep->clk_enet_out);
>  failed_clk_enet_out:
> - clk_disable_unprepare(fep->clk_ahb);
> + clk_disable_unprepare(fep->clk_ahb);
> 
>   return ret;
>  }
> --
> 2.1.4

RE: [PATCH 2/3] net: fec: convert to using gpiod framework

[Duan Andy]

From: Lothar Waßmann  Sent: Monday, November 30, 2015 
7:33 PM
> To: Andrew Lunn; David S. Miller; Estevam Fabio-R49496; Greg Ungerer;
> Kevin Hao; Lothar Waßmann; Lucas Stach; Duan Fugang-B38611; Philippe
> Reynes; Richard Cochran; Russell King; Sascha Hauer; Stefan Agner; linux-
> ker...@vger.kernel.org; net...@vger.kernel.org; Jeff Kirsher; Uwe Kleine-
> König
> Subject: [PATCH 2/3] net: fec: convert to using gpiod framework
> 
> Use gpiod_get_optional() instead of checking for a valid GPIO number and
> calling devm_gpio_request_one() conditionally.
> 
> Signed-off-by: Lothar Waßmann 
> ---
>  drivers/net/ethernet/freescale/fec_main.c | 17 +++--
>  1 file changed, 7 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/fec_main.c
> b/drivers/net/ethernet/freescale/fec_main.c
> index e17d74f..1a983fc 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -3233,7 +3233,7 @@ static int fec_enet_init(struct net_device *ndev)
> #ifdef CONFIG_OF  static void fec_reset_phy(struct platform_device *pdev)
> {
> - int err, phy_reset;
> + struct gpio_desc *phy_reset;
>   int msec = 1;
>   struct device_node *np = pdev->dev.of_node;
> 
> @@ -3245,18 +3245,15 @@ static void fec_reset_phy(struct platform_device
> *pdev)
>   if (msec > 1000)
>   msec = 1;
> 
> - phy_reset = of_get_named_gpio(np, "phy-reset-gpios", 0);
> - if (!gpio_is_valid(phy_reset))
> - return;
> -
> - err = devm_gpio_request_one(>dev, phy_reset,
> - GPIOF_OUT_INIT_LOW, "phy-reset");
> - if (err) {
> - dev_err(>dev, "failed to get phy-reset-gpios: %d\n",
> err);
> + phy_reset = devm_gpiod_get_optional(>dev, "phy-reset",
> + GPIOD_OUT_LOW);
> + if (IS_ERR(phy_reset)) {
> + dev_err(>dev, "failed to get phy-reset-gpios: %ld\n",
> + PTR_ERR(phy_reset));
>   return;
>   }
>   msleep(msec);
> - gpio_set_value_cansleep(phy_reset, 1);
> + gpiod_set_value_cansleep(phy_reset, 1);

This API will judge the GPIO active polarity, there many imx boards in dts 
files don't care the polarity.
So pls drop the patch.

Or use gpiod_set_raw_value_cansleep() instead of gpiod_set_value_cansleep().

>  }
>  #else /* CONFIG_OF */
>  static void fec_reset_phy(struct platform_device *pdev)
> --
> 2.1.4

RE: [PATCH 3/3] net: fec: Reset ethernet PHY whenever the enet_out clock is being enabled

[Duan Andy]

From: Lothar Waßmann  Sent: Monday, November 30, 2015 
7:33 PM
> To: Andrew Lunn; David S. Miller; Estevam Fabio-R49496; Greg Ungerer;
> Kevin Hao; Lothar Waßmann; Lucas Stach; Duan Fugang-B38611; Philippe
> Reynes; Richard Cochran; Russell King; Sascha Hauer; Stefan Agner; linux-
> ker...@vger.kernel.org; net...@vger.kernel.org; Jeff Kirsher; Uwe Kleine-
> König
> Subject: [PATCH 3/3] net: fec: Reset ethernet PHY whenever the enet_out
> clock is being enabled
> 
> If a PHY uses ENET_OUT as reference clock, it may need a RESET to get
> functional after the clock had been disabled.
> 
> Failure to do this results in the link state constantly toggling between
> up and down:
> fec 02188000.ethernet eth0: Link is Up - 100Mbps/Full - flow control
> rx/tx fec 02188000.ethernet eth0: Link is Down fec 02188000.ethernet eth0:
> Link is Up - 100Mbps/Full - flow control rx/tx fec 02188000.ethernet eth0:
> Link is Down [...]
> 
> Signed-off-by: Lothar Waßmann 
> ---
>  drivers/net/ethernet/freescale/fec.h  |  1 +
>  drivers/net/ethernet/freescale/fec_main.c | 46 -
> --
>  2 files changed, 37 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/fec.h
> b/drivers/net/ethernet/freescale/fec.h
> index 99d33e2..8ab4f7f 100644
> --- a/drivers/net/ethernet/freescale/fec.h
> +++ b/drivers/net/ethernet/freescale/fec.h
> @@ -519,6 +519,7 @@ struct fec_enet_private {
>   int pause_flag;
>   int wol_flag;
>   u32 quirks;
> + struct gpio_desc *phy_reset;
> 
>   struct  napi_struct napi;
>   int csum_flags;
> diff --git a/drivers/net/ethernet/freescale/fec_main.c
> b/drivers/net/ethernet/freescale/fec_main.c
> index 1a983fc..7ba2bbb 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -66,6 +66,7 @@
> 
>  static void set_multicast_list(struct net_device *ndev);  static void
> fec_enet_itr_coal_init(struct net_device *ndev);
> +static void fec_reset_phy(struct platform_device *pdev);
> 
>  #define DRIVER_NAME  "fec"
> 
> @@ -1861,6 +1862,8 @@ static int fec_enet_clk_enable(struct net_device
> *ndev, bool enable)
>   ret = clk_prepare_enable(fep->clk_enet_out);
>   if (ret)
>   goto failed_clk_enet_out;
> +
> + fec_reset_phy(fep->pdev);
>   }
>   if (fep->clk_ptp) {
>   mutex_lock(>ptp_clk_mutex);
> @@ -3231,13 +3234,32 @@ static int fec_enet_init(struct net_device
> *ndev)  }
> 
>  #ifdef CONFIG_OF
> -static void fec_reset_phy(struct platform_device *pdev)
> +static struct gpio_desc *fec_get_reset_gpio(struct platform_device
> +*pdev)
>  {
>   struct gpio_desc *phy_reset;
> - int msec = 1;
>   struct device_node *np = pdev->dev.of_node;
> 
>   if (!np)
> + return ERR_PTR(-ENODEV);

No need to check device node.

> +
> + phy_reset = devm_gpiod_get_optional(>dev, "phy-reset",
> + GPIOD_OUT_LOW);
> + if (IS_ERR(phy_reset))
> + dev_err(>dev, "failed to get phy-reset-gpios: %ld\n",
> + PTR_ERR(phy_reset));
> + return phy_reset;
> +}
> +
> +static void fec_reset_phy(struct platform_device *pdev) {
> + struct device_node *np = pdev->dev.of_node;
> + struct net_device *ndev = platform_get_drvdata(pdev);
> + struct fec_enet_private *fep = netdev_priv(ndev);
> + int msec = 1;
> +
> + if (!fep->phy_reset)
> + return;
> + if (!np)
>   return;

No need to check device node.

> 
>   of_property_read_u32(np, "phy-reset-duration", ); @@ -3245,17
> +3267,16 @@ static void fec_reset_phy(struct platform_device *pdev)
>   if (msec > 1000)
>   msec = 1;
> 
> - phy_reset = devm_gpiod_get_optional(>dev, "phy-reset",
> - GPIOD_OUT_LOW);
> - if (IS_ERR(phy_reset)) {
> - dev_err(>dev, "failed to get phy-reset-gpios: %ld\n",
> - PTR_ERR(phy_reset));
> - return;
> - }
> + gpiod_set_value_cansleep(fep->phy_reset, 0);
>   msleep(msec);
> - gpiod_set_value_cansleep(phy_reset, 1);
> + gpiod_set_value_cansleep(fep->phy_reset, 1);
>  }
>  #else /* CONFIG_OF */
> +static void fec_get_reset_gpio(struct platform_device *pdev) {
> + return -EINVAL;
> +}
> +
>  static void fec_reset_phy(struct platform_device *pdev)  {
>   /*
> @@ -3309,8 +3330,12 @@ fec_probe(struct platform_device *pdev)
>   struct device_node *np = pdev->dev.of_node, *phy_node;
>   int num_tx_qs;
>   int num_rx_qs;
> + struct gpio_desc *phy_reset;
> 
>   fec_enet_get_queue_num(pdev, _tx_qs, _rx_qs);
> + phy_reset = fec_get_reset_gpio(pdev);
> + if (IS_ERR(phy_reset) && PTR_ERR(phy_reset) == -EPROBE_DEFER)
> + return -EPROBE_DEFER;

RE: [PATCH 0/3] net: fec: Reset ethernet PHY whenever the enet_out clock

[Duan Andy]

From: Lothar Waßmann  Sent: Tuesday, December 01, 
2015 3:25 PM
> To: Duan Fugang-B38611
> Cc: Andrew Lunn; David S. Miller; Estevam Fabio-R49496; Greg Ungerer;
> Kevin Hao; Lucas Stach; Philippe Reynes; Richard Cochran; Russell King;
> Sascha Hauer; Stefan Agner; linux-kernel@vger.kernel.org;
> net...@vger.kernel.org; Jeff Kirsher; Uwe Kleine-K?nig
> Subject: Re: [PATCH 0/3] net: fec: Reset ethernet PHY whenever the
> enet_out clock
> 
> Hi,
> 
> > From: Lothar Waßmann  Sent: Monday, November
> > 30, 2015 7:33 PM
> > > To: Andrew Lunn; David S. Miller; Estevam Fabio-R49496; Greg
> > > Ungerer; Kevin Hao; Lothar Waßmann; Lucas Stach; Duan Fugang-B38611;
> > > Philippe Reynes; Richard Cochran; Russell King; Sascha Hauer; Stefan
> > > Agner; linux- ker...@vger.kernel.org; net...@vger.kernel.org; Jeff
> > > Kirsher; Uwe Kleine- König
> > > Subject: [PATCH 0/3] net: fec: Reset ethernet PHY whenever the
> > > enet_out clock
> > >
> > > This patchset fixes a regression introduced by commit 8fff755e9f8d
> ("net:
> > > fec: Ensure clocks are enabled while using mdio bus") for ethernet
> > > PHYs that are using ENET_OUT as reference clock (on i.MX6 or i.MX28)
> > >
> > Do you mean commit 8fff755e9f8d cause your problem ?  This commit just
> manage ipg clock in runtime because mdio bus can access external phy
> switch no matter netdev status.
> >
> No. Actually I meant commit e8fcfcd5684a ("net: fec: optimize the clock
> management to save power") which started to disable the clocks when not
> in use.
> 
Understand. Yes, disable enet_out clock for power saving is necessary.

> Sorry for the confusion,
> Lothar Waßmann

RE: [PATCH] net: fec: fix enet_out clock handling

[Duan Andy]

From: Lothar Waßmann  Sent: Monday, November 30, 2015 
2:56 PM
> To: Duan Fugang-B38611
> Cc: Andrew Lunn; David S. Miller; Estevam Fabio-R49496; Kevin Hao; Lucas
> Stach; Philippe Reynes; Russell King; Uwe Kleine-K?nig; linux-
> ker...@vger.kernel.org; net...@vger.kernel.org; Stefan Agner
> Subject: Re: [PATCH] net: fec: fix enet_out clock handling
> 
> Hi,
> 
> > From: Lothar Waßmann  Sent: Friday, November
> > 27, 2015 9:39 PM
> > > To: Andrew Lunn; David S. Miller; Estevam Fabio-R49496; Kevin Hao;
> > > Lothar Waßmann; Lucas Stach; Duan Fugang-B38611; Philippe Reynes;
> > > Russell King; Uwe Kleine-König; linux-kernel@vger.kernel.org;
> > > net...@vger.kernel.org; Stefan Agner
> > > Subject: [PATCH] net: fec: fix enet_out clock handling
> > >
> > > When ENET_OUT is being used as reference clock for an external PHY,
> > > the clock must not be disabled while the PHY is active. Otherwise
> > > the PHY may lose its internal state and require a reset to become
> functional again.
> > >
> > > A symptom for this bug is a network interface that constantly
> > > toggles between UP and DOWN state:
> > > fec 800f.ethernet eth0: Link is Up - 100Mbps/Full - flow control
> > > rx/tx fec 800f.ethernet eth0: Link is Down fec 800f.ethernet
> eth0:
> > > Link is Up - 100Mbps/Full - flow control rx/tx fec 800f.ethernet
> eth0:
> > > Link is Down [...]
> > >
> > > Signed-off-by: Lothar Waßmann 
> > > ---
> > >  drivers/net/ethernet/freescale/fec_main.c | 34
> > > +
> > > --
> > >  1 file changed, 14 insertions(+), 20 deletions(-)
> > >
> >
> > When MAC is not ready with clocks disabled,  it is not necessary to
> supply clock for PHY. In fact, PHY also is not ready, why does it need
> clock ?
> > For your problem, you must add PHY reset in your dts file to resolve
> your problem.
> >
> The phy-reset-gpio property is set in the DTB. But fec_reset_phy() which
> asserts the RESET is only called from within the probe() function.
> It should probably be called from fec_restart() instead?
> 
After enet_out clock enable, you can call fec_reset_phy() do phy reset.  Don't 
put it in .fec_restart() function because
Cable hotplug test cause phy registers reset to HW default status.

Regards,
Andy
N�r��yb�X��ǧv�^�)޺{.n�+{zX����ܨ}���Ơz�:+v���zZ+��+zf���h���~i���z��w���?�&�)ߢf��^jǫy�m��@A�a���
0��h���i

RE: [PATCH] net: fec: fix enet_out clock handling

[Duan Andy]

From: Andrew Lunn  Sent: Sunday, November 29, 2015 12:44 AM
> To: Duan Fugang-B38611
> Cc: Lothar Wa?mann; David S. Miller; Estevam Fabio-R49496; Kevin Hao;
> Lucas Stach; Philippe Reynes; Russell King; Uwe Kleine-K?nig; linux-
> ker...@vger.kernel.org; net...@vger.kernel.org; Stefan Agner
> Subject: Re: [PATCH] net: fec: fix enet_out clock handling
> 
> > When MAC is not ready with clocks disabled, it is not necessary to
> > supply clock for PHY. In fact, PHY also is not ready, why does it need
> > clock ?
> 
> How about the case of the "PHY" is actually a switch? You can use the
> MDIO bus separate from the MAC, you can configure the switch while the
> MAC is down, etc. Packets can be flowing in and out of switch ports,
> while the MAC is down. You only need the MAC up when the host wants to
> send packets.
> 
>  Andrew

Do you mean PHY switch also use the enet_out clock ?
- If not, this topic is not related to the patch.
- If so, we should add flag check, we can disable the enet_out clock when the 
phy is not switch or switch phy doesn't use the clock in real case, which can 
save power.  And the patch title is not right, not "fix" since it is not a 
issue, just different usage for different cases.

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

RE: [PATCH] net: fec: fix enet_out clock handling

[Duan Andy]

From: Andrew Lunn  Sent: Sunday, November 29, 2015 12:44 AM
> To: Duan Fugang-B38611
> Cc: Lothar Wa?mann; David S. Miller; Estevam Fabio-R49496; Kevin Hao;
> Lucas Stach; Philippe Reynes; Russell King; Uwe Kleine-K?nig; linux-
> ker...@vger.kernel.org; net...@vger.kernel.org; Stefan Agner
> Subject: Re: [PATCH] net: fec: fix enet_out clock handling
> 
> > When MAC is not ready with clocks disabled, it is not necessary to
> > supply clock for PHY. In fact, PHY also is not ready, why does it need
> > clock ?
> 
> How about the case of the "PHY" is actually a switch? You can use the
> MDIO bus separate from the MAC, you can configure the switch while the
> MAC is down, etc. Packets can be flowing in and out of switch ports,
> while the MAC is down. You only need the MAC up when the host wants to
> send packets.
> 
>  Andrew

Do you mean PHY switch also use the enet_out clock ?
- If not, this topic is not related to the patch.
- If so, we should add flag check, we can disable the enet_out clock when the 
phy is not switch or switch phy doesn't use the clock in real case, which can 
save power.  And the patch title is not right, not "fix" since it is not a 
issue, just different usage for different cases.

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

RE: [PATCH] net: fec: fix enet_out clock handling

[Duan Andy]

From: Lothar Waßmann  Sent: Monday, November 30, 2015 
2:56 PM
> To: Duan Fugang-B38611
> Cc: Andrew Lunn; David S. Miller; Estevam Fabio-R49496; Kevin Hao; Lucas
> Stach; Philippe Reynes; Russell King; Uwe Kleine-K?nig; linux-
> ker...@vger.kernel.org; net...@vger.kernel.org; Stefan Agner
> Subject: Re: [PATCH] net: fec: fix enet_out clock handling
> 
> Hi,
> 
> > From: Lothar Waßmann  Sent: Friday, November
> > 27, 2015 9:39 PM
> > > To: Andrew Lunn; David S. Miller; Estevam Fabio-R49496; Kevin Hao;
> > > Lothar Waßmann; Lucas Stach; Duan Fugang-B38611; Philippe Reynes;
> > > Russell King; Uwe Kleine-König; linux-kernel@vger.kernel.org;
> > > net...@vger.kernel.org; Stefan Agner
> > > Subject: [PATCH] net: fec: fix enet_out clock handling
> > >
> > > When ENET_OUT is being used as reference clock for an external PHY,
> > > the clock must not be disabled while the PHY is active. Otherwise
> > > the PHY may lose its internal state and require a reset to become
> functional again.
> > >
> > > A symptom for this bug is a network interface that constantly
> > > toggles between UP and DOWN state:
> > > fec 800f.ethernet eth0: Link is Up - 100Mbps/Full - flow control
> > > rx/tx fec 800f.ethernet eth0: Link is Down fec 800f.ethernet
> eth0:
> > > Link is Up - 100Mbps/Full - flow control rx/tx fec 800f.ethernet
> eth0:
> > > Link is Down [...]
> > >
> > > Signed-off-by: Lothar Waßmann 
> > > ---
> > >  drivers/net/ethernet/freescale/fec_main.c | 34
> > > +
> > > --
> > >  1 file changed, 14 insertions(+), 20 deletions(-)
> > >
> >
> > When MAC is not ready with clocks disabled,  it is not necessary to
> supply clock for PHY. In fact, PHY also is not ready, why does it need
> clock ?
> > For your problem, you must add PHY reset in your dts file to resolve
> your problem.
> >
> The phy-reset-gpio property is set in the DTB. But fec_reset_phy() which
> asserts the RESET is only called from within the probe() function.
> It should probably be called from fec_restart() instead?
> 
After enet_out clock enable, you can call fec_reset_phy() do phy reset.  Don't 
put it in .fec_restart() function because
Cable hotplug test cause phy registers reset to HW default status.

Regards,
Andy
N�r��yb�X��ǧv�^�)޺{.n�+{zX����ܨ}���Ơz�:+v���zZ+��+zf���h���~i���z��w���?�&�)ߢf��^jǫy�m��@A�a���
0��h���i

RE: [PATCH] net: fec: fix enet_out clock handling

[Duan Andy]

From: Lothar Waßmann  Sent: Friday, November 27, 2015 
9:39 PM
> To: Andrew Lunn; David S. Miller; Estevam Fabio-R49496; Kevin Hao; Lothar
> Waßmann; Lucas Stach; Duan Fugang-B38611; Philippe Reynes; Russell King;
> Uwe Kleine-König; linux-kernel@vger.kernel.org; net...@vger.kernel.org;
> Stefan Agner
> Subject: [PATCH] net: fec: fix enet_out clock handling
> 
> When ENET_OUT is being used as reference clock for an external PHY, the
> clock must not be disabled while the PHY is active. Otherwise the PHY may
> lose its internal state and require a reset to become functional again.
> 
> A symptom for this bug is a network interface that constantly toggles
> between UP and DOWN state:
> fec 800f.ethernet eth0: Link is Up - 100Mbps/Full - flow control
> rx/tx fec 800f.ethernet eth0: Link is Down fec 800f.ethernet eth0:
> Link is Up - 100Mbps/Full - flow control rx/tx fec 800f.ethernet eth0:
> Link is Down [...]
> 
> Signed-off-by: Lothar Waßmann 
> ---
>  drivers/net/ethernet/freescale/fec_main.c | 34 +
> --
>  1 file changed, 14 insertions(+), 20 deletions(-)
> 

When MAC is not ready with clocks disabled,  it is not necessary to supply 
clock for PHY. In fact, PHY also is not ready, why does it need clock ?
For your problem, you must add PHY reset in your dts file to resolve your 
problem.

I don't agree with this patch.

> diff --git a/drivers/net/ethernet/freescale/fec_main.c
> b/drivers/net/ethernet/freescale/fec_main.c
> index d2328fc..d9df4c5 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -1857,11 +1857,6 @@ static int fec_enet_clk_enable(struct net_device
> *ndev, bool enable)
>   ret = clk_prepare_enable(fep->clk_ahb);
>   if (ret)
>   return ret;
> - if (fep->clk_enet_out) {
> - ret = clk_prepare_enable(fep->clk_enet_out);
> - if (ret)
> - goto failed_clk_enet_out;
> - }
>   if (fep->clk_ptp) {
>   mutex_lock(>ptp_clk_mutex);
>   ret = clk_prepare_enable(fep->clk_ptp); @@ -1873,35
> +1868,26 @@ static int fec_enet_clk_enable(struct net_device *ndev, bool
> enable)
>   }
>   mutex_unlock(>ptp_clk_mutex);
>   }
> - if (fep->clk_ref) {
> - ret = clk_prepare_enable(fep->clk_ref);
> - if (ret)
> - goto failed_clk_ref;
> - }
> + ret = clk_prepare_enable(fep->clk_ref);
> + if (ret)
> + goto failed_clk_ref;
>   } else {
>   clk_disable_unprepare(fep->clk_ahb);
> - if (fep->clk_enet_out)
> - clk_disable_unprepare(fep->clk_enet_out);
>   if (fep->clk_ptp) {
>   mutex_lock(>ptp_clk_mutex);
>   clk_disable_unprepare(fep->clk_ptp);
>   fep->ptp_clk_on = false;
>   mutex_unlock(>ptp_clk_mutex);
>   }
> - if (fep->clk_ref)
> - clk_disable_unprepare(fep->clk_ref);
> + clk_disable_unprepare(fep->clk_ref);
>   }
> 
>   return 0;
> 
>  failed_clk_ref:
> - if (fep->clk_ref)
> - clk_disable_unprepare(fep->clk_ref);
> + clk_disable_unprepare(fep->clk_ref);
>  failed_clk_ptp:
> - if (fep->clk_enet_out)
> - clk_disable_unprepare(fep->clk_enet_out);
> -failed_clk_enet_out:
> - clk_disable_unprepare(fep->clk_ahb);
> + clk_disable_unprepare(fep->clk_ahb);
> 
>   return ret;
>  }
> @@ -3425,6 +3411,10 @@ fec_probe(struct platform_device *pdev)
>   if (ret)
>   goto failed_clk;
> 
> + ret = clk_prepare_enable(fep->clk_enet_out);
> + if (ret)
> + goto failed_clk_enet_out;
> +
>   ret = clk_prepare_enable(fep->clk_ipg);
>   if (ret)
>   goto failed_clk_ipg;
> @@ -3509,6 +3499,8 @@ failed_init:
>   if (fep->reg_phy)
>   regulator_disable(fep->reg_phy);
>  failed_regulator:
> + clk_disable_unprepare(fep->clk_enet_out);
> +failed_clk_enet_out:
>   clk_disable_unprepare(fep->clk_ipg);
>  failed_clk_ipg:
>   fec_enet_clk_enable(ndev, false);
> @@ -3531,6 +3523,8 @@ fec_drv_remove(struct platform_device *pdev)
>   fec_ptp_stop(pdev);
>   unregister_netdev(ndev);
>   fec_enet_mii_remove(fep);
> + fec_enet_clk_enable(ndev, false);
> + clk_disable_unprepare(fep->clk_enet_out);
>   if (fep->reg_phy)
>   regulator_disable(fep->reg_phy);
>   of_node_put(fep->phy_node);
> --
> 2.1.4

RE: [PATCH] net: fec: fix enet_out clock handling

[Duan Andy]

From: Lothar Waßmann  Sent: Friday, November 27, 2015 
9:39 PM
> To: Andrew Lunn; David S. Miller; Estevam Fabio-R49496; Kevin Hao; Lothar
> Waßmann; Lucas Stach; Duan Fugang-B38611; Philippe Reynes; Russell King;
> Uwe Kleine-König; linux-kernel@vger.kernel.org; net...@vger.kernel.org;
> Stefan Agner
> Subject: [PATCH] net: fec: fix enet_out clock handling
> 
> When ENET_OUT is being used as reference clock for an external PHY, the
> clock must not be disabled while the PHY is active. Otherwise the PHY may
> lose its internal state and require a reset to become functional again.
> 
> A symptom for this bug is a network interface that constantly toggles
> between UP and DOWN state:
> fec 800f.ethernet eth0: Link is Up - 100Mbps/Full - flow control
> rx/tx fec 800f.ethernet eth0: Link is Down fec 800f.ethernet eth0:
> Link is Up - 100Mbps/Full - flow control rx/tx fec 800f.ethernet eth0:
> Link is Down [...]
> 
> Signed-off-by: Lothar Waßmann 
> ---
>  drivers/net/ethernet/freescale/fec_main.c | 34 +
> --
>  1 file changed, 14 insertions(+), 20 deletions(-)
> 

When MAC is not ready with clocks disabled,  it is not necessary to supply 
clock for PHY. In fact, PHY also is not ready, why does it need clock ?
For your problem, you must add PHY reset in your dts file to resolve your 
problem.

I don't agree with this patch.

> diff --git a/drivers/net/ethernet/freescale/fec_main.c
> b/drivers/net/ethernet/freescale/fec_main.c
> index d2328fc..d9df4c5 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -1857,11 +1857,6 @@ static int fec_enet_clk_enable(struct net_device
> *ndev, bool enable)
>   ret = clk_prepare_enable(fep->clk_ahb);
>   if (ret)
>   return ret;
> - if (fep->clk_enet_out) {
> - ret = clk_prepare_enable(fep->clk_enet_out);
> - if (ret)
> - goto failed_clk_enet_out;
> - }
>   if (fep->clk_ptp) {
>   mutex_lock(>ptp_clk_mutex);
>   ret = clk_prepare_enable(fep->clk_ptp); @@ -1873,35
> +1868,26 @@ static int fec_enet_clk_enable(struct net_device *ndev, bool
> enable)
>   }
>   mutex_unlock(>ptp_clk_mutex);
>   }
> - if (fep->clk_ref) {
> - ret = clk_prepare_enable(fep->clk_ref);
> - if (ret)
> - goto failed_clk_ref;
> - }
> + ret = clk_prepare_enable(fep->clk_ref);
> + if (ret)
> + goto failed_clk_ref;
>   } else {
>   clk_disable_unprepare(fep->clk_ahb);
> - if (fep->clk_enet_out)
> - clk_disable_unprepare(fep->clk_enet_out);
>   if (fep->clk_ptp) {
>   mutex_lock(>ptp_clk_mutex);
>   clk_disable_unprepare(fep->clk_ptp);
>   fep->ptp_clk_on = false;
>   mutex_unlock(>ptp_clk_mutex);
>   }
> - if (fep->clk_ref)
> - clk_disable_unprepare(fep->clk_ref);
> + clk_disable_unprepare(fep->clk_ref);
>   }
> 
>   return 0;
> 
>  failed_clk_ref:
> - if (fep->clk_ref)
> - clk_disable_unprepare(fep->clk_ref);
> + clk_disable_unprepare(fep->clk_ref);
>  failed_clk_ptp:
> - if (fep->clk_enet_out)
> - clk_disable_unprepare(fep->clk_enet_out);
> -failed_clk_enet_out:
> - clk_disable_unprepare(fep->clk_ahb);
> + clk_disable_unprepare(fep->clk_ahb);
> 
>   return ret;
>  }
> @@ -3425,6 +3411,10 @@ fec_probe(struct platform_device *pdev)
>   if (ret)
>   goto failed_clk;
> 
> + ret = clk_prepare_enable(fep->clk_enet_out);
> + if (ret)
> + goto failed_clk_enet_out;
> +
>   ret = clk_prepare_enable(fep->clk_ipg);
>   if (ret)
>   goto failed_clk_ipg;
> @@ -3509,6 +3499,8 @@ failed_init:
>   if (fep->reg_phy)
>   regulator_disable(fep->reg_phy);
>  failed_regulator:
> + clk_disable_unprepare(fep->clk_enet_out);
> +failed_clk_enet_out:
>   clk_disable_unprepare(fep->clk_ipg);
>  failed_clk_ipg:
>   fec_enet_clk_enable(ndev, false);
> @@ -3531,6 +3523,8 @@ fec_drv_remove(struct platform_device *pdev)
>   fec_ptp_stop(pdev);
>   unregister_netdev(ndev);
>   fec_enet_mii_remove(fep);
> + fec_enet_clk_enable(ndev, false);
> + clk_disable_unprepare(fep->clk_enet_out);
>   if (fep->reg_phy)
>   regulator_disable(fep->reg_phy);
>   of_node_put(fep->phy_node);
> --
> 2.1.4

RE: [PATCH v1] Revert "serial: imx: remove unbalanced clk_prepare"

[Duan Andy]

From: Robin Gong  Sent: Wednesday, November 04, 2015 
10:58 AM
> To: gre...@linuxfoundation.org; edubez...@gmail.com
> Cc: Duan Fugang-B38611; linux-ser...@vger.kernel.org; linux-
> ker...@vger.kernel.org
> Subject: [PATCH v1] Revert "serial: imx: remove unbalanced clk_prepare"
> 
> commit 9e7b399d6528 ("serial: imx: remove unbalanced clk_prepare").
> Otherwise below warning happen since there are some printk logs in
> interrupt.
> 
> [   14.868319] udevd[501]: starting version 182
> [   16.386107] random: nonblocking pool is initialized
> [   16.386123] [ cut here ]
> [   16.386140] WARNING: CPU: 0 PID: 501 at kernel/locking/mutex.c:868
> mutex_trylock+0x210/0x230()
> [   16.386146] DEBUG_LOCKS_WARN_ON(in_interrupt())
> [   16.386149] Modules linked in:
> [   16.386157] CPU: 0 PID: 501 Comm: udevd Not tainted 4.3.0-rc1-00014-
> gf843df8 #28
> [   16.386160] Hardware name: Freescale i.MX6 SoloX (Device Tree)
> [   16.386165] Backtrace:
> [   16.386182] [] (dump_backtrace) from []
> (show_stack+0x18/0x1c)
> [   16.386192]  r6:c0af1840 r5: r4: r3:
> [   16.386201] [] (show_stack) from []
> (dump_stack+0x8c/0xa4)
> [   16.386216] [] (dump_stack) from []
> (warn_slowpath_common+0x80/0xbc)
> [   16.386225]  r6:c07b1ccc r5:0009 r4:edcf5c60 r3:0001
> [   16.386234] [] (warn_slowpath_common) from []
> (warn_slowpath_fmt+0x38/0x40)
> [   16.386246]  r8:c0564694 r7:eeb76880 r6:c13323ec r5:0001
> r4:c0976990
> [   16.386255] [] (warn_slowpath_fmt) from []
> (mutex_trylock+0x210/0x230)
> [   16.386261]  r3:c0978f50 r2:c0976990
> [   16.386264]  r4:c0b29a70
> [   16.386278] [] (mutex_trylock) from []
> (clk_prepare_lock+0x14/0xf4)
> [   16.386289]  r8:c133000c r7:eeb76880 r6:0037 r5:c12efbc8
> r4:eeb76880
> [   16.386297] [] (clk_prepare_lock) from []
> (clk_prepare+0x18/0x38)
> [   16.386303]  r5:c12efbc8 r4:eeb76880
> [   16.386311] [] (clk_prepare) from []
> (imx_console_write+0x34/0x248)
> [   16.386317]  r4:ee999c10 r3:c134a92c
> [   16.386329] [] (imx_console_write) from []
> (call_console_drivers.constprop.25+0xe0/0x104)
> [   16.386342]  r10:c07b938c r9: r8:c133000c r7:0037
> r6:edcf4000 r5:c12ef6c0
> [   16.386345]  r4:c0afef38
> [   16.386355] [] (call_console_drivers.constprop.25) from
> [] (console_unlock+0x3fc/0x57c)
> [   16.386367]  r10:c132fff0 r9:0100 r8:0037 r7:
> r6:0005 r5:c12f4df0
> 
> Signed-off-by: Robin Gong 
> ---
>  drivers/tty/serial/imx.c | 20 ++--
>  1 file changed, 14 insertions(+), 6 deletions(-)

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

RE: [PATCH v1] Revert "serial: imx: remove unbalanced clk_prepare"

[Duan Andy]

From: Robin Gong  Sent: Wednesday, November 04, 2015 
10:58 AM
> To: gre...@linuxfoundation.org; edubez...@gmail.com
> Cc: Duan Fugang-B38611; linux-ser...@vger.kernel.org; linux-
> ker...@vger.kernel.org
> Subject: [PATCH v1] Revert "serial: imx: remove unbalanced clk_prepare"
> 
> commit 9e7b399d6528 ("serial: imx: remove unbalanced clk_prepare").
> Otherwise below warning happen since there are some printk logs in
> interrupt.
> 
> [   14.868319] udevd[501]: starting version 182
> [   16.386107] random: nonblocking pool is initialized
> [   16.386123] [ cut here ]
> [   16.386140] WARNING: CPU: 0 PID: 501 at kernel/locking/mutex.c:868
> mutex_trylock+0x210/0x230()
> [   16.386146] DEBUG_LOCKS_WARN_ON(in_interrupt())
> [   16.386149] Modules linked in:
> [   16.386157] CPU: 0 PID: 501 Comm: udevd Not tainted 4.3.0-rc1-00014-
> gf843df8 #28
> [   16.386160] Hardware name: Freescale i.MX6 SoloX (Device Tree)
> [   16.386165] Backtrace:
> [   16.386182] [] (dump_backtrace) from []
> (show_stack+0x18/0x1c)
> [   16.386192]  r6:c0af1840 r5: r4: r3:
> [   16.386201] [] (show_stack) from []
> (dump_stack+0x8c/0xa4)
> [   16.386216] [] (dump_stack) from []
> (warn_slowpath_common+0x80/0xbc)
> [   16.386225]  r6:c07b1ccc r5:0009 r4:edcf5c60 r3:0001
> [   16.386234] [] (warn_slowpath_common) from []
> (warn_slowpath_fmt+0x38/0x40)
> [   16.386246]  r8:c0564694 r7:eeb76880 r6:c13323ec r5:0001
> r4:c0976990
> [   16.386255] [] (warn_slowpath_fmt) from []
> (mutex_trylock+0x210/0x230)
> [   16.386261]  r3:c0978f50 r2:c0976990
> [   16.386264]  r4:c0b29a70
> [   16.386278] [] (mutex_trylock) from []
> (clk_prepare_lock+0x14/0xf4)
> [   16.386289]  r8:c133000c r7:eeb76880 r6:0037 r5:c12efbc8
> r4:eeb76880
> [   16.386297] [] (clk_prepare_lock) from []
> (clk_prepare+0x18/0x38)
> [   16.386303]  r5:c12efbc8 r4:eeb76880
> [   16.386311] [] (clk_prepare) from []
> (imx_console_write+0x34/0x248)
> [   16.386317]  r4:ee999c10 r3:c134a92c
> [   16.386329] [] (imx_console_write) from []
> (call_console_drivers.constprop.25+0xe0/0x104)
> [   16.386342]  r10:c07b938c r9: r8:c133000c r7:0037
> r6:edcf4000 r5:c12ef6c0
> [   16.386345]  r4:c0afef38
> [   16.386355] [] (call_console_drivers.constprop.25) from
> [] (console_unlock+0x3fc/0x57c)
> [   16.386367]  r10:c132fff0 r9:0100 r8:0037 r7:
> r6:0005 r5:c12f4df0
> 
> Signed-off-by: Robin Gong 
> ---
>  drivers/tty/serial/imx.c | 20 ++--
>  1 file changed, 14 insertions(+), 6 deletions(-)

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

Re: [PATCH] iio: adc: vf610_adc: Fix division by zero error

[Duan Andy]

From: Sanchayan Maity  Sent: Monday, October 19, 2015 
3:43 PM
>To: ji...@kernel.org
>Cc: ste...@agner.ch; Duan Fugang-B38611; linux-...@vger.kernel.org; 
>linux->ker...@vger.kernel.org; Sanchayan Maity
>Subject: [PATCH] iio: adc: vf610_adc: Fix division by zero error
>
>In case the fsl,adck-max-frequency property is not present in
>the device tree, a division by zero error results during the
>probe call on kernel boot (see below). This patch fixes it and
>also restores device tree compatibility in case kernels are
>booting with old device trees without this property specified.
>
>[1.063229] Division by zero in kernel.
>[1.067152] CPU: 0 PID: 1 Comm: swapper Not tainted
>4.3.0-rc5-00212-gcc88cef #37
>[1.074650] Hardware name: Freescale Vybrid VF5xx/VF6xx (Device Tree)
>[1.081135] Backtrace:
>[1.083694] [<800134a4>] (dump_backtrace) from [<8001369c>]
>(show_stack+0x18/0x1c)
>[1.091340]  r7:0008 r6:8e0ae210 r5: r4:8e299800
>[1.097146] [<80013684>] (show_stack) from [<80297b1c>]
>(dump_stack+0x24/0x28)
>[1.104483] [<80297af8>] (dump_stack) from [<80013608>]
>(__div0+0x1c/0x20)
>[1.111421] [<800135ec>] (__div0) from [<802968b4>] (Ldiv0+0x8/0x10)
>[1.117865] [<80424350>] (vf610_adc_probe) from [<803153b4>]
>(platform_drv_probe+0x4c/0xac)
>[1.126311]  r10: r9:8076a5ec r8: r7:fdfb
>r6:807cc67c r5:8e0ae210
>[1.134319]  r4:807f6c54
>[1.136915] [<80315368>] (platform_drv_probe) from [<803138bc>]
>(driver_probe_device+0x20c/0x2f8)
>[1.145882]  r7:807cc67c r6: r5:8e0ae210 r4:807f6c54
>[1.151657] [<803136b0>] (driver_probe_device) from [<80313a3c>]
>(__driver_attach+0x94/0x98)
>[1.160190]  r9:8076a5ec r8:0098 r7: r6:8e0ae244
>r5:807cc67c r4:8e0ae210
>[1.168112] [<803139a8>] (__driver_attach) from [<80311cb8>]
>(bus_for_each_dev+0x70/0xa4)
>[1.176383]  r7: r6:803139a8 r5:807cc67c r4:
>[1.182159] [<80311c48>] (bus_for_each_dev) from [<80313318>]
>(driver_attach+0x24/0x28)
>[1.190260]  r6:807bb568 r5:8e2a5b00 r4:807cc67c
>[1.194996] [<803132f4>] (driver_attach) from [<80312f50>]
>(bus_add_driver+0x1a4/0x21c)
>[1.203113] [<80312dac>] (bus_add_driver) from [<803142a8>]
>(driver_register+0x80/0x100)
>[1.211275]  r7:8e2a7dc0 r6:807a8160 r5:80789e14 r4:807cc67c
>[1.217075] [<80314228>] (driver_register) from [<803152f8>]
>(__platform_driver_register+0x5c/0x64)
>[1.226216]  r5:80789e14 r4:807a8160
>[1.229877] [<8031529c>] (__platform_driver_register) from
>[<80789e30>] (vf610_adc_driver_init+0x1c/0x20)
>[1.239556] [<80789e14>] (vf610_adc_driver_init) from [<800095f8>]
>(do_one_initcall+0x94/0x1dc)
>[1.248365] [<80009564>] (do_one_initcall) from [<8076ae34>]
>(kernel_init_freeable+0x13c/0x1e0)
>[1.257155]  r10:80794830 r9:8076a5ec r8:0098 r7:807d5780
>r6:807d5780 r5:0006
>[1.265153]  r4:807a0ee8
>[1.267753] [<8076acf8>] (kernel_init_freeable) from [<80590ef0>]
>(kernel_init+0x18/0xf0)
>[1.276021]  r10: r9: r8: r7:
>r6: r5:80590ed8
>[1.284015]  r4:807d5780
>[1.286615] [<80590ed8>] (kernel_init) from [<8000f878>]
>(ret_from_fork+0x14/0x3c)
>[1.294278]  r5:80590ed8 r4:
>
>Signed-off-by: Sanchayan Maity 

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

Re: [PATCH] iio: adc: vf610_adc: Fix division by zero error

[Duan Andy]

From: Sanchayan Maity  Sent: Monday, October 19, 2015 
3:43 PM
>To: ji...@kernel.org
>Cc: ste...@agner.ch; Duan Fugang-B38611; linux-...@vger.kernel.org; 
>linux->ker...@vger.kernel.org; Sanchayan Maity
>Subject: [PATCH] iio: adc: vf610_adc: Fix division by zero error
>
>In case the fsl,adck-max-frequency property is not present in
>the device tree, a division by zero error results during the
>probe call on kernel boot (see below). This patch fixes it and
>also restores device tree compatibility in case kernels are
>booting with old device trees without this property specified.
>
>[1.063229] Division by zero in kernel.
>[1.067152] CPU: 0 PID: 1 Comm: swapper Not tainted
>4.3.0-rc5-00212-gcc88cef #37
>[1.074650] Hardware name: Freescale Vybrid VF5xx/VF6xx (Device Tree)
>[1.081135] Backtrace:
>[1.083694] [<800134a4>] (dump_backtrace) from [<8001369c>]
>(show_stack+0x18/0x1c)
>[1.091340]  r7:0008 r6:8e0ae210 r5: r4:8e299800
>[1.097146] [<80013684>] (show_stack) from [<80297b1c>]
>(dump_stack+0x24/0x28)
>[1.104483] [<80297af8>] (dump_stack) from [<80013608>]
>(__div0+0x1c/0x20)
>[1.111421] [<800135ec>] (__div0) from [<802968b4>] (Ldiv0+0x8/0x10)
>[1.117865] [<80424350>] (vf610_adc_probe) from [<803153b4>]
>(platform_drv_probe+0x4c/0xac)
>[1.126311]  r10: r9:8076a5ec r8: r7:fdfb
>r6:807cc67c r5:8e0ae210
>[1.134319]  r4:807f6c54
>[1.136915] [<80315368>] (platform_drv_probe) from [<803138bc>]
>(driver_probe_device+0x20c/0x2f8)
>[1.145882]  r7:807cc67c r6: r5:8e0ae210 r4:807f6c54
>[1.151657] [<803136b0>] (driver_probe_device) from [<80313a3c>]
>(__driver_attach+0x94/0x98)
>[1.160190]  r9:8076a5ec r8:0098 r7: r6:8e0ae244
>r5:807cc67c r4:8e0ae210
>[1.168112] [<803139a8>] (__driver_attach) from [<80311cb8>]
>(bus_for_each_dev+0x70/0xa4)
>[1.176383]  r7: r6:803139a8 r5:807cc67c r4:
>[1.182159] [<80311c48>] (bus_for_each_dev) from [<80313318>]
>(driver_attach+0x24/0x28)
>[1.190260]  r6:807bb568 r5:8e2a5b00 r4:807cc67c
>[1.194996] [<803132f4>] (driver_attach) from [<80312f50>]
>(bus_add_driver+0x1a4/0x21c)
>[1.203113] [<80312dac>] (bus_add_driver) from [<803142a8>]
>(driver_register+0x80/0x100)
>[1.211275]  r7:8e2a7dc0 r6:807a8160 r5:80789e14 r4:807cc67c
>[1.217075] [<80314228>] (driver_register) from [<803152f8>]
>(__platform_driver_register+0x5c/0x64)
>[1.226216]  r5:80789e14 r4:807a8160
>[1.229877] [<8031529c>] (__platform_driver_register) from
>[<80789e30>] (vf610_adc_driver_init+0x1c/0x20)
>[1.239556] [<80789e14>] (vf610_adc_driver_init) from [<800095f8>]
>(do_one_initcall+0x94/0x1dc)
>[1.248365] [<80009564>] (do_one_initcall) from [<8076ae34>]
>(kernel_init_freeable+0x13c/0x1e0)
>[1.257155]  r10:80794830 r9:8076a5ec r8:0098 r7:807d5780
>r6:807d5780 r5:0006
>[1.265153]  r4:807a0ee8
>[1.267753] [<8076acf8>] (kernel_init_freeable) from [<80590ef0>]
>(kernel_init+0x18/0xf0)
>[1.276021]  r10: r9: r8: r7:
>r6: r5:80590ed8
>[1.284015]  r4:807d5780
>[1.286615] [<80590ed8>] (kernel_init) from [<8000f878>]
>(ret_from_fork+0x14/0x3c)
>[1.294278]  r5:80590ed8 r4:
>
>Signed-off-by: Sanchayan Maity 

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

RE: [PATCH] vf610_adc: Fix internal temperature calculation

[Duan Andy]

From: Jonathan Cameron  Sent: Sunday, September 27, 2015 
11:53 PM
> To: Bhuvanchandra DV; linux-...@vger.kernel.org
> Cc: ste...@agner.ch; maitysancha...@gmail.com; Duan Fugang-B38611;
> knaac...@gmx.de; l...@metafoo.de; pme...@pmeerw.net; shawn@linaro.org;
> linux-kernel@vger.kernel.org; Duan Fugang-B38611
> Subject: Re: [PATCH] vf610_adc: Fix internal temperature calculation
> 
> On 23/09/15 14:43, Bhuvanchandra DV wrote:
> > There is an observed temperature difference of ~20°C with the internal
> > temperature reading and the temperature measured on SoC package.
> > Existing calculations consider the typical values provided in
> > datasheet. Those typical values are valid for VREFH_ADC at 3.0V.
> > Voltage at 25°C is different for different VREFH_ADC voltages. With
> > VREFH_ADC at 3.3V, voltage at 25°C is 0.699V. Hence update the VTEMP25
> > to 0.699V which gives ADCR@Temp25 as 867 and the final temperature
> > readings differs with ~5°C from the external readings.
> >
> > Formula for finding ADCR@Temp25:
> > ADCR@Temp25 = (ADCR@Vdd * V@TEMP25 * 10) / VDDconv
> >
> > ADCR@Vdd for 12-Bit ADC = 4095
> > VDDconv = VREFH_ADC * 10
> >
> > VREFH_ADC@3.3V
> > ADCR@Temp25 = (4095 * .699 * 10) / 33
> > ADCR@Temp25 ~= 867
> >
> > | VREFH_ADC | V@TEMP25 | VDDconv | ADCR@Temp25 |
> > |   3.0V| 0.696mV  |30   | 950 |
> > |   3.3V| 0.699mV  |33   | 867 |
> >
> > Signed-off-by: Bhuvanchandra DV 
> Looks fine to me, but I'll need an Ack from Fugang on this one as I don't
> know or have the part I'm afraid.
> 
> Jonathan

The patch is fine for me. Sorry for the late response due to vacation.

Acked-by: Fugang Duan 

> > ---
> >  drivers/iio/adc/vf610_adc.c | 19 ++-
> >  1 file changed, 14 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/iio/adc/vf610_adc.c b/drivers/iio/adc/vf610_adc.c
> > index f4df2a7..e7abc13 100644
> > --- a/drivers/iio/adc/vf610_adc.c
> > +++ b/drivers/iio/adc/vf610_adc.c
> > @@ -103,6 +103,13 @@
> >
> >  #define DEFAULT_SAMPLE_TIME1000
> >
> > +/* V at 25°C of 696 mV */
> > +#define VF610_VTEMP25_3V0  950
> > +/* V at 25°C of 699 mV */
> > +#define VF610_VTEMP25_3V3  867
> > +/* Typical sensor slope coefficient at all temperatures */
> > +#define VF610_TEMP_SLOPE_COEFF 1840
> > +
> >  enum clk_sel {
> > VF610_ADCIOC_BUSCLK_SET,
> > VF610_ADCIOC_ALTCLK_SET,
> > @@ -636,11 +643,13 @@ static int vf610_read_raw(struct iio_dev
> *indio_dev,
> > break;
> > case IIO_TEMP:
> > /*
> > -   * Calculate in degree Celsius times 1000
> > -   * Using sensor slope of 1.84 mV/°C and
> > -   * V at 25°C of 696 mV
> > -   */
> > -   *val = 25000 - ((int)info->value - 864) * 100 /
> 1840;
> > +* Calculate in degree Celsius times 1000
> > +* Using the typical sensor slope of 1.84 mV/°C
> > +* and VREFH_ADC at 3.3V, V at 25°C of 699 mV
> > +*/
> > +   *val = 25000 - ((int)info->value - VF610_VTEMP25_3V3) *
> > +   100 / VF610_TEMP_SLOPE_COEFF;
> > +
> > break;
> > default:
> > mutex_unlock(_dev->mlock);
> >
N�r��yb�X��ǧv�^�)޺{.n�+{zX����ܨ}���Ơz�:+v���zZ+��+zf���h���~i���z��w���?�&�)ߢf��^jǫy�m��@A�a���
0��h���i

RE: [PATCH] vf610_adc: Fix internal temperature calculation

[Duan Andy]

From: Jonathan Cameron  Sent: Sunday, September 27, 2015 
11:53 PM
> To: Bhuvanchandra DV; linux-...@vger.kernel.org
> Cc: ste...@agner.ch; maitysancha...@gmail.com; Duan Fugang-B38611;
> knaac...@gmx.de; l...@metafoo.de; pme...@pmeerw.net; shawn@linaro.org;
> linux-kernel@vger.kernel.org; Duan Fugang-B38611
> Subject: Re: [PATCH] vf610_adc: Fix internal temperature calculation
> 
> On 23/09/15 14:43, Bhuvanchandra DV wrote:
> > There is an observed temperature difference of ~20°C with the internal
> > temperature reading and the temperature measured on SoC package.
> > Existing calculations consider the typical values provided in
> > datasheet. Those typical values are valid for VREFH_ADC at 3.0V.
> > Voltage at 25°C is different for different VREFH_ADC voltages. With
> > VREFH_ADC at 3.3V, voltage at 25°C is 0.699V. Hence update the VTEMP25
> > to 0.699V which gives ADCR@Temp25 as 867 and the final temperature
> > readings differs with ~5°C from the external readings.
> >
> > Formula for finding ADCR@Temp25:
> > ADCR@Temp25 = (ADCR@Vdd * V@TEMP25 * 10) / VDDconv
> >
> > ADCR@Vdd for 12-Bit ADC = 4095
> > VDDconv = VREFH_ADC * 10
> >
> > VREFH_ADC@3.3V
> > ADCR@Temp25 = (4095 * .699 * 10) / 33
> > ADCR@Temp25 ~= 867
> >
> > | VREFH_ADC | V@TEMP25 | VDDconv | ADCR@Temp25 |
> > |   3.0V| 0.696mV  |30   | 950 |
> > |   3.3V| 0.699mV  |33   | 867 |
> >
> > Signed-off-by: Bhuvanchandra DV 
> Looks fine to me, but I'll need an Ack from Fugang on this one as I don't
> know or have the part I'm afraid.
> 
> Jonathan

The patch is fine for me. Sorry for the late response due to vacation.

Acked-by: Fugang Duan 

> > ---
> >  drivers/iio/adc/vf610_adc.c | 19 ++-
> >  1 file changed, 14 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/iio/adc/vf610_adc.c b/drivers/iio/adc/vf610_adc.c
> > index f4df2a7..e7abc13 100644
> > --- a/drivers/iio/adc/vf610_adc.c
> > +++ b/drivers/iio/adc/vf610_adc.c
> > @@ -103,6 +103,13 @@
> >
> >  #define DEFAULT_SAMPLE_TIME1000
> >
> > +/* V at 25°C of 696 mV */
> > +#define VF610_VTEMP25_3V0  950
> > +/* V at 25°C of 699 mV */
> > +#define VF610_VTEMP25_3V3  867
> > +/* Typical sensor slope coefficient at all temperatures */
> > +#define VF610_TEMP_SLOPE_COEFF 1840
> > +
> >  enum clk_sel {
> > VF610_ADCIOC_BUSCLK_SET,
> > VF610_ADCIOC_ALTCLK_SET,
> > @@ -636,11 +643,13 @@ static int vf610_read_raw(struct iio_dev
> *indio_dev,
> > break;
> > case IIO_TEMP:
> > /*
> > -   * Calculate in degree Celsius times 1000
> > -   * Using sensor slope of 1.84 mV/°C and
> > -   * V at 25°C of 696 mV
> > -   */
> > -   *val = 25000 - ((int)info->value - 864) * 100 /
> 1840;
> > +* Calculate in degree Celsius times 1000
> > +* Using the typical sensor slope of 1.84 mV/°C
> > +* and VREFH_ADC at 3.3V, V at 25°C of 699 mV
> > +*/
> > +   *val = 25000 - ((int)info->value - VF610_VTEMP25_3V3) *
> > +   100 / VF610_TEMP_SLOPE_COEFF;
> > +
> > break;
> > default:
> > mutex_unlock(_dev->mlock);
> >
N�r��yb�X��ǧv�^�)޺{.n�+{zX����ܨ}���Ơz�:+v���zZ+��+zf���h���~i���z��w���?�&�)ߢf��^jǫy�m��@A�a���
0��h���i

RE: [PATCH v4] iio: adc: vf610: Add IIO buffer support for Vybrid ADC

[Duan Andy]

From: Sanchayan Maity  Sent: Monday, August 17, 2015 
11:52 PM
> To: ji...@kernel.org; linux-...@vger.kernel.org
> Cc: ste...@agner.ch; Duan Fugang-B38611; pme...@pmeerw.net; linux-
> ker...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; Sanchayan
> Maity
> Subject: [PATCH v4] iio: adc: vf610: Add IIO buffer support for Vybrid
> ADC
> 
> This patch adds support for IIO buffer to the Vybrid ADC driver.
> IIO triggered buffer infrastructure along with iio sysfs trigger is used
> to leverage continuous sampling support provided by the ADC block.
> 
> Signed-off-by: Sanchayan Maity 
> ---
> 
> Changes since v3:
> Fix iio_buffer_setup_ops for postenable and predisable functions to match
> pairwise. Before this the predisable work was being done in postdisable.
> 
> Changes since v2:
> 1. Fix the wrong buffer size for statically allocated buffer 2. Drop the
> use of .address field from the iio_chan_spec 3. Use iio_buffer_enabled
> call inside the lock 4. Drop wrapper function around iio_trigered_*
> function calls 5. Drop Kconfig select of sysfs trigger 6. Drop Kconfig
> select IIO_TRIGGER as it is already selected by IIO_TRIGGERED_BUFFER
> 
> Changes since v1:
> 1. Use a fixed size buffer instead of kmalloc allocated during update
> scan mode 2. Remove a write to read only register ADC_HS (COCO bit)
> 
>  drivers/iio/adc/Kconfig |   2 +
>  drivers/iio/adc/vf610_adc.c | 105
> +---
>  2 files changed, 100 insertions(+), 7 deletions(-)
> 

The version is fine for me. Thanks for your effort.

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

RE: [PATCH v4] iio: adc: vf610: Add IIO buffer support for Vybrid ADC

[Duan Andy]

From: Sanchayan Maity maitysancha...@gmail.com Sent: Monday, August 17, 2015 
11:52 PM
 To: ji...@kernel.org; linux-...@vger.kernel.org
 Cc: ste...@agner.ch; Duan Fugang-B38611; pme...@pmeerw.net; linux-
 ker...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; Sanchayan
 Maity
 Subject: [PATCH v4] iio: adc: vf610: Add IIO buffer support for Vybrid
 ADC
 
 This patch adds support for IIO buffer to the Vybrid ADC driver.
 IIO triggered buffer infrastructure along with iio sysfs trigger is used
 to leverage continuous sampling support provided by the ADC block.
 
 Signed-off-by: Sanchayan Maity maitysancha...@gmail.com
 ---
 
 Changes since v3:
 Fix iio_buffer_setup_ops for postenable and predisable functions to match
 pairwise. Before this the predisable work was being done in postdisable.
 
 Changes since v2:
 1. Fix the wrong buffer size for statically allocated buffer 2. Drop the
 use of .address field from the iio_chan_spec 3. Use iio_buffer_enabled
 call inside the lock 4. Drop wrapper function around iio_trigered_*
 function calls 5. Drop Kconfig select of sysfs trigger 6. Drop Kconfig
 select IIO_TRIGGER as it is already selected by IIO_TRIGGERED_BUFFER
 
 Changes since v1:
 1. Use a fixed size buffer instead of kmalloc allocated during update
 scan mode 2. Remove a write to read only register ADC_HS (COCO bit)
 
  drivers/iio/adc/Kconfig |   2 +
  drivers/iio/adc/vf610_adc.c | 105
 +---
  2 files changed, 100 insertions(+), 7 deletions(-)
 

The version is fine for me. Thanks for your effort.

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

RE: [PATCH] iio: adc: vf610: Add IIO buffer support for Vybrid ADC

[Duan Andy]

From: Sanchayan Maity  Sent: Monday, August 03, 2015 
11:10 PM
> To: ji...@kernel.org; linux-...@vger.kernel.org
> Cc: ste...@agner.ch; Duan Fugang-B38611; hof...@osadl.org;
> sanjeev_sha...@mentor.com; Estevam Fabio-R49496; knaac...@gmx.de;
> l...@metafoo.de; pme...@pmeerw.net; antoine.ten...@free-electrons.com;
> linux-kernel@vger.kernel.org; linux-arm-ker...@lists.infradead.org;
> Sanchayan Maity
> Subject: [PATCH] iio: adc: vf610: Add IIO buffer support for Vybrid ADC
> 
> This patch adds support for IIO buffer to the Vybrid ADC driver.
> IIO triggered buffer infrastructure along with iio sysfs trigger is used
> to leverage continuous sampling support provided by the ADC block.
> 
> Signed-off-by: Sanchayan Maity 
> ---
>  drivers/iio/adc/Kconfig |   4 ++
>  drivers/iio/adc/vf610_adc.c | 122
> +---
>  2 files changed, 120 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig index
> 7c55658..4661241 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -337,6 +337,10 @@ config TWL6030_GPADC  config VF610_ADC
>   tristate "Freescale vf610 ADC driver"
>   depends on OF
> + select IIO_BUFFER
> + select IIO_TRIGGER
> + select IIO_SYSFS_TRIGGER
> + select IIO_TRIGGERED_BUFFER
>   help
> Say yes here to support for Vybrid board analog-to-digital
> converter.
> Since the IP is used for i.MX6SLX, the driver also support
> i.MX6SLX.
> diff --git a/drivers/iio/adc/vf610_adc.c b/drivers/iio/adc/vf610_adc.c
> index 23b8fb9..af72b9a 100644
> --- a/drivers/iio/adc/vf610_adc.c
> +++ b/drivers/iio/adc/vf610_adc.c
> @@ -34,8 +34,11 @@
>  #include 
> 
>  #include 
> +#include 
>  #include 
> -#include 
> +#include 
> +#include  #include
> +
> 
>  /* This will be the driver name the kernel reports */  #define
> DRIVER_NAME "vf610-adc"
> @@ -170,6 +173,7 @@ struct vf610_adc {
>   u32 sample_freq_avail[5];
> 
>   struct completion completion;
> + u16 *buffer;
>  };
> 
>  static const u32 vf610_hw_avgs[] = { 1, 4, 8, 16, 32 }; @@ -505,12
> +509,22 @@ static const struct iio_chan_spec_ext_info vf610_ext_info[] =
> {
>   .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |  \
>   BIT(IIO_CHAN_INFO_SAMP_FREQ),   \
>   .ext_info = vf610_ext_info, \
> + .address = (_idx),  \
> + .scan_index = (_idx),   \
> + .scan_type.sign = 'u',  \
> + .scan_type.realbits = 12,   \
> + .scan_type.storagebits = 16,\
>  }
> 
>  #define VF610_ADC_TEMPERATURE_CHAN(_idx, _chan_type) {   \
>   .type = (_chan_type),   \
>   .channel = (_idx),  \
>   .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED), \
> + .address = (_idx),  \
> + .scan_index = (_idx),   \
> + .scan_type.sign = 'u',  \
> + .scan_type.realbits = 12,   \
> + .scan_type.storagebits = 16,\
>  }
> 
>  static const struct iio_chan_spec vf610_adc_iio_channels[] = { @@ -531,6
> +545,7 @@ static const struct iio_chan_spec vf610_adc_iio_channels[] = {
>   VF610_ADC_CHAN(14, IIO_VOLTAGE),
>   VF610_ADC_CHAN(15, IIO_VOLTAGE),
>   VF610_ADC_TEMPERATURE_CHAN(26, IIO_TEMP),
> + IIO_CHAN_SOFT_TIMESTAMP(32),
>   /* sentinel */
>  };
> 
> @@ -559,13 +574,21 @@ static int vf610_adc_read_data(struct vf610_adc
> *info)
> 
>  static irqreturn_t vf610_adc_isr(int irq, void *dev_id)  {
> - struct vf610_adc *info = (struct vf610_adc *)dev_id;
> + struct iio_dev *indio_dev = (struct iio_dev *)dev_id;
> + struct vf610_adc *info = iio_priv(indio_dev);
>   int coco;
> 
>   coco = readl(info->regs + VF610_REG_ADC_HS);
>   if (coco & VF610_ADC_HS_COCO0) {
>   info->value = vf610_adc_read_data(info);
> - complete(>completion);
> + if (iio_buffer_enabled(indio_dev)) {
> + info->buffer[0] = info->value;
> + writel(0, info->regs + VF610_REG_ADC_HS);
The register is read only. After ADC_Rn is read, the coco bit is cleared.

> + iio_push_to_buffers_with_timestamp(indio_dev,
> + info->buffer, iio_get_time_ns());
> + iio_trigger_notify_done(indio_dev->trig);
> + } else
> + complete(>completion);
>   }
> 
>   return IRQ_HANDLED;
> @@ -612,6 +635,9 @@ static int vf610_read_raw(struct iio_dev *indio_dev,
>   switch (mask) {
>   case IIO_CHAN_INFO_RAW:
>   case IIO_CHAN_INFO_PROCESSED:
> + if (iio_buffer_enabled(indio_dev))
> + return -EBUSY;
> +
>   mutex_lock(_dev->mlock);
>   

RE: [PATCH] iio: adc: vf610: Add IIO buffer support for Vybrid ADC

[Duan Andy]

From: Sanchayan Maity maitysancha...@gmail.com Sent: Monday, August 03, 2015 
11:10 PM
 To: ji...@kernel.org; linux-...@vger.kernel.org
 Cc: ste...@agner.ch; Duan Fugang-B38611; hof...@osadl.org;
 sanjeev_sha...@mentor.com; Estevam Fabio-R49496; knaac...@gmx.de;
 l...@metafoo.de; pme...@pmeerw.net; antoine.ten...@free-electrons.com;
 linux-kernel@vger.kernel.org; linux-arm-ker...@lists.infradead.org;
 Sanchayan Maity
 Subject: [PATCH] iio: adc: vf610: Add IIO buffer support for Vybrid ADC
 
 This patch adds support for IIO buffer to the Vybrid ADC driver.
 IIO triggered buffer infrastructure along with iio sysfs trigger is used
 to leverage continuous sampling support provided by the ADC block.
 
 Signed-off-by: Sanchayan Maity maitysancha...@gmail.com
 ---
  drivers/iio/adc/Kconfig |   4 ++
  drivers/iio/adc/vf610_adc.c | 122
 +---
  2 files changed, 120 insertions(+), 6 deletions(-)
 
 diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig index
 7c55658..4661241 100644
 --- a/drivers/iio/adc/Kconfig
 +++ b/drivers/iio/adc/Kconfig
 @@ -337,6 +337,10 @@ config TWL6030_GPADC  config VF610_ADC
   tristate Freescale vf610 ADC driver
   depends on OF
 + select IIO_BUFFER
 + select IIO_TRIGGER
 + select IIO_SYSFS_TRIGGER
 + select IIO_TRIGGERED_BUFFER
   help
 Say yes here to support for Vybrid board analog-to-digital
 converter.
 Since the IP is used for i.MX6SLX, the driver also support
 i.MX6SLX.
 diff --git a/drivers/iio/adc/vf610_adc.c b/drivers/iio/adc/vf610_adc.c
 index 23b8fb9..af72b9a 100644
 --- a/drivers/iio/adc/vf610_adc.c
 +++ b/drivers/iio/adc/vf610_adc.c
 @@ -34,8 +34,11 @@
  #include linux/err.h
 
  #include linux/iio/iio.h
 +#include linux/iio/buffer.h
  #include linux/iio/sysfs.h
 -#include linux/iio/driver.h
 +#include linux/iio/trigger.h
 +#include linux/iio/trigger_consumer.h #include
 +linux/iio/triggered_buffer.h
 
  /* This will be the driver name the kernel reports */  #define
 DRIVER_NAME vf610-adc
 @@ -170,6 +173,7 @@ struct vf610_adc {
   u32 sample_freq_avail[5];
 
   struct completion completion;
 + u16 *buffer;
  };
 
  static const u32 vf610_hw_avgs[] = { 1, 4, 8, 16, 32 }; @@ -505,12
 +509,22 @@ static const struct iio_chan_spec_ext_info vf610_ext_info[] =
 {
   .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |  \
   BIT(IIO_CHAN_INFO_SAMP_FREQ),   \
   .ext_info = vf610_ext_info, \
 + .address = (_idx),  \
 + .scan_index = (_idx),   \
 + .scan_type.sign = 'u',  \
 + .scan_type.realbits = 12,   \
 + .scan_type.storagebits = 16,\
  }
 
  #define VF610_ADC_TEMPERATURE_CHAN(_idx, _chan_type) {   \
   .type = (_chan_type),   \
   .channel = (_idx),  \
   .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED), \
 + .address = (_idx),  \
 + .scan_index = (_idx),   \
 + .scan_type.sign = 'u',  \
 + .scan_type.realbits = 12,   \
 + .scan_type.storagebits = 16,\
  }
 
  static const struct iio_chan_spec vf610_adc_iio_channels[] = { @@ -531,6
 +545,7 @@ static const struct iio_chan_spec vf610_adc_iio_channels[] = {
   VF610_ADC_CHAN(14, IIO_VOLTAGE),
   VF610_ADC_CHAN(15, IIO_VOLTAGE),
   VF610_ADC_TEMPERATURE_CHAN(26, IIO_TEMP),
 + IIO_CHAN_SOFT_TIMESTAMP(32),
   /* sentinel */
  };
 
 @@ -559,13 +574,21 @@ static int vf610_adc_read_data(struct vf610_adc
 *info)
 
  static irqreturn_t vf610_adc_isr(int irq, void *dev_id)  {
 - struct vf610_adc *info = (struct vf610_adc *)dev_id;
 + struct iio_dev *indio_dev = (struct iio_dev *)dev_id;
 + struct vf610_adc *info = iio_priv(indio_dev);
   int coco;
 
   coco = readl(info-regs + VF610_REG_ADC_HS);
   if (coco  VF610_ADC_HS_COCO0) {
   info-value = vf610_adc_read_data(info);
 - complete(info-completion);
 + if (iio_buffer_enabled(indio_dev)) {
 + info-buffer[0] = info-value;
 + writel(0, info-regs + VF610_REG_ADC_HS);
The register is read only. After ADC_Rn is read, the coco bit is cleared.

 + iio_push_to_buffers_with_timestamp(indio_dev,
 + info-buffer, iio_get_time_ns());
 + iio_trigger_notify_done(indio_dev-trig);
 + } else
 + complete(info-completion);
   }
 
   return IRQ_HANDLED;
 @@ -612,6 +635,9 @@ static int vf610_read_raw(struct iio_dev *indio_dev,
   switch (mask) {
   case IIO_CHAN_INFO_RAW:
   case IIO_CHAN_INFO_PROCESSED:
 + if (iio_buffer_enabled(indio_dev))
 + return -EBUSY;
 +

RE: Buggy cable detection on i.MX51, fec driver and LAN8700 PHY

[Duan Andy]

From: Igor Plyatov  Sent: Monday, July 27, 2015 10:51 PM
> To: linux-kernel@vger.kernel.org; net...@vger.kernel.org
> Cc: Florian Fainelli; Joe Perches; Zhou Luwei-B45643; Duan Fugang-B38611;
> Richard Cochran; David S. Miller; Uwe Kleine-König; Estevam Fabio-R49496;
> Lothar Waßmann; Li Frank-B20596
> Subject: Buggy cable detection on i.MX51, fec driver and LAN8700 PHY
> 
> Dear all,
> 
> very often we observe issue with Ethernet cable detection during cable
> unplugging and plugging.
> 
> We use Voipac i.MX51 SOMs (System On Modules). They are based on
> Freescale i.MX51 CPU with LAN7800 PHY in MII mode. The schematic of PHY
> connection is very similar to the Freescale i.MX51 Babbage board.
> 
> The Ethernet interface eth0 is configured statically for simplicity, but
> same issue exists with DHCP configuration.
> 
> I did a lot of tests to determine stability of Ethernet cable detection
> by the "fec" Ethernet driver.
> 
> In normal operation, if I unplug the Ethernet cable, then "fec" driver
> prints "fec 83fec000.ethernet eth0: Link is Down" and green LED (Ethernet
> medium detected) is OFF.
> If I plug cable back, then "fec" driver print "fec 83fec000.ethernet
> eth0: Link is Up - 100Mbps/Full - flow control off" and green LED is ON.
> 
> But sometimes, after cable plugging, "fec" driver does not print anything
> on the console and green LED does not show detection of Ethernet cable.
> Frequency of issue appearing is a random value.
> Sometimes issue appears after second cable unplugging/plugging, but
> sometimes - after 10-20 unplugging/plugging.
> 
> The issue was tested and exists on kernels from linux-3.8.5 till current
> linux-4.2-rc4-cbfe8fa6cd672011c755c3cd85c9ffd4e2d10a6f.
> 
> Same tests was made with different versions of the Barebox bootloader and
> cable detection works flawless.
> 
> Please, help to resolve issue with Linux drivers.
> 
> Best wishes.
> --
> Igor Plyatov

For phy cable detection issue, mostly it is phy device issue like:
-   Phy is not stable, maybe the phy initial setting is not right.
-   MDIO bus read is not right, maybe MDIO/MDC pin drive strength (timing) 
doesn't match spec requirement.

You can measure the MDIO timing firstly, and then check the phy setting.

Regards,
Andy

RE: Buggy cable detection on i.MX51, fec driver and LAN8700 PHY

[Duan Andy]

From: Igor Plyatov plya...@gmail.com Sent: Monday, July 27, 2015 10:51 PM
 To: linux-kernel@vger.kernel.org; net...@vger.kernel.org
 Cc: Florian Fainelli; Joe Perches; Zhou Luwei-B45643; Duan Fugang-B38611;
 Richard Cochran; David S. Miller; Uwe Kleine-König; Estevam Fabio-R49496;
 Lothar Waßmann; Li Frank-B20596
 Subject: Buggy cable detection on i.MX51, fec driver and LAN8700 PHY
 
 Dear all,
 
 very often we observe issue with Ethernet cable detection during cable
 unplugging and plugging.
 
 We use Voipac i.MX51 SOMs (System On Modules). They are based on
 Freescale i.MX51 CPU with LAN7800 PHY in MII mode. The schematic of PHY
 connection is very similar to the Freescale i.MX51 Babbage board.
 
 The Ethernet interface eth0 is configured statically for simplicity, but
 same issue exists with DHCP configuration.
 
 I did a lot of tests to determine stability of Ethernet cable detection
 by the fec Ethernet driver.
 
 In normal operation, if I unplug the Ethernet cable, then fec driver
 prints fec 83fec000.ethernet eth0: Link is Down and green LED (Ethernet
 medium detected) is OFF.
 If I plug cable back, then fec driver print fec 83fec000.ethernet
 eth0: Link is Up - 100Mbps/Full - flow control off and green LED is ON.
 
 But sometimes, after cable plugging, fec driver does not print anything
 on the console and green LED does not show detection of Ethernet cable.
 Frequency of issue appearing is a random value.
 Sometimes issue appears after second cable unplugging/plugging, but
 sometimes - after 10-20 unplugging/plugging.
 
 The issue was tested and exists on kernels from linux-3.8.5 till current
 linux-4.2-rc4-cbfe8fa6cd672011c755c3cd85c9ffd4e2d10a6f.
 
 Same tests was made with different versions of the Barebox bootloader and
 cable detection works flawless.
 
 Please, help to resolve issue with Linux drivers.
 
 Best wishes.
 --
 Igor Plyatov

For phy cable detection issue, mostly it is phy device issue like:
-   Phy is not stable, maybe the phy initial setting is not right.
-   MDIO bus read is not right, maybe MDIO/MDC pin drive strength (timing) 
doesn't match spec requirement.

You can measure the MDIO timing firstly, and then check the phy setting.

Regards,
Andy

RE: [PATCH 3.19.y-ckt 173/251] clocksource: exynos_mct: Avoid blocking calls in the cpu hotplug notifier

[Duan Andy]

From: Krzysztof Kozlowski  Sent: Thursday, July 16, 
2015 5:52 PM
> To: Duan Fugang-B38611
> Cc: Krzysztof Kozlowski; Kamal Mostafa; daniel.lezc...@linaro.org; linux-
> ker...@vger.kernel.org; sta...@vger.kernel.org; Damian Eppel;
> kg...@kernel.org; kyungmin.p...@samsung.com; Thomas Gleixner;
> m.szyprow...@samsung.com; linux-arm-ker...@lists.infradead.org; kernel-
> t...@lists.ubuntu.com
> Subject: Re: [PATCH 3.19.y-ckt 173/251] clocksource: exynos_mct: Avoid
> blocking calls in the cpu hotplug notifier
> 
> 2015-07-16 17:14 GMT+09:00 Duan Andy :
> > From: Krzysztof Kozlowski  Sent: Thursday,
> > July 16, 2015 2:56 PM
> >> To: Duan Fugang-B38611
> >> Cc: Kamal Mostafa; linux-kernel@vger.kernel.org;
> >> sta...@vger.kernel.org; kernel-t...@lists.ubuntu.com;
> >> daniel.lezc...@linaro.org; Damian Eppel; kyungmin.p...@samsung.com;
> >> kg...@kernel.org; Thomas Gleixner; linux-arm-
> >> ker...@lists.infradead.org; m.szyprow...@samsung.com
> >> Subject: Re: [PATCH 3.19.y-ckt 173/251] clocksource: exynos_mct:
> >> Avoid blocking calls in the cpu hotplug notifier
> >>
> >> 2015-07-16 15:37 GMT+09:00 Duan Andy :
> >> > From: Kamal Mostafa  Sent: Thursday, July 16,
> >> > 2015 9:08 AM
> >> >> To: linux-kernel@vger.kernel.org; sta...@vger.kernel.org; kernel-
> >> >> t...@lists.ubuntu.com
> >> >> Cc: Kamal Mostafa; daniel.lezc...@linaro.org;
> >> >> kyungmin.p...@samsung.com; Damian Eppel; kg...@kernel.org; Thomas
> >> >> Gleixner; linux-arm- ker...@lists.infradead.org;
> >> >> m.szyprow...@samsung.com
> >> >> Subject: [PATCH 3.19.y-ckt 173/251] clocksource: exynos_mct: Avoid
> >> >> blocking calls in the cpu hotplug notifier
> >> >>
> >> >> 3.19.8-ckt4 -stable review patch.  If anyone has any objections,
> >> >> please let me know.
> >> >>
> >> >> --
> >> >>
> >> >> From: Damian Eppel 
> >> >>
> >> >> commit 56a94f13919c0db5958611b388e1581b4852f3c9 upstream.
> >> >>
> >> >> Whilst testing cpu hotplug events on kernel configured with
> >> >> DEBUG_PREEMPT and DEBUG_ATOMIC_SLEEP we get following BUG message,
> >> >> caused by calling
> >> >> request_irq() and free_irq() in the context of hotplug
> >> >> notification (which is in this case atomic context).
> >> >>
> >> >> [   40.785859] CPU1: Software reset
> >> >> [   40.786660] BUG: sleeping function called from invalid context
> at
> >> >> mm/slub.c:1241
> >> >> [   40.786668] in_atomic(): 1, irqs_disabled(): 128, pid: 0, name:
> >> >> swapper/1
> >> >> [   40.786678] Preemption disabled at:[<  (null)>]   (null)
> >> >> [   40.786681]
> >> >> [   40.786692] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 3.19.0-
> rc4-
> >> >> 00024-g7dca860 #36
> >> >> [   40.786698] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
> >> >> [   40.786728] [] (unwind_backtrace) from []
> >> >> (show_stack+0x10/0x14)
> >> >> [   40.786747] [] (show_stack) from []
> >> >> (dump_stack+0x70/0xbc)
> >> >> [   40.786767] [] (dump_stack) from []
> >> >> (kmem_cache_alloc+0xd8/0x170)
> >> >> [   40.786785] [] (kmem_cache_alloc) from []
> >> >> (request_threaded_irq+0x64/0x128)
> >> >> [   40.786804] [] (request_threaded_irq) from []
> >> >> (exynos4_local_timer_setup+0xc0/0x13c)
> >> >> [   40.786820] [] (exynos4_local_timer_setup) from
> >> []
> >> >> (exynos4_mct_cpu_notify+0x30/0xa8)
> >> >> [   40.786838] [] (exynos4_mct_cpu_notify) from
> []
> >> >> (notifier_call_chain+0x44/0x84)
> >> >> [   40.786857] [] (notifier_call_chain) from []
> >> >> (__cpu_notify+0x28/0x44)
> >> >> [   40.786873] [] (__cpu_notify) from []
> >> >> (secondary_start_kernel+0xec/0x150)
> >> >> [   40.786886] [] (secondary_start_kernel) from
> [<40008764>]
> >> >> (0x40008764)
> >> >>
> >> >> Interrupts cannot be requested/freed in the CPU_STARTING/CPU_DYING
> >> >> notifications which run on the hotplugged cpu with interrupts and
> >> >> preemption disabled.
> >> >>
> >> >> To avoid the issue, request the interrupts for all p

RE: [PATCH 3.19.y-ckt 173/251] clocksource: exynos_mct: Avoid blocking calls in the cpu hotplug notifier

[Duan Andy]

From: Krzysztof Kozlowski  Sent: Thursday, July 16, 
2015 2:56 PM
> To: Duan Fugang-B38611
> Cc: Kamal Mostafa; linux-kernel@vger.kernel.org; sta...@vger.kernel.org;
> kernel-t...@lists.ubuntu.com; daniel.lezc...@linaro.org; Damian Eppel;
> kyungmin.p...@samsung.com; kg...@kernel.org; Thomas Gleixner; linux-arm-
> ker...@lists.infradead.org; m.szyprow...@samsung.com
> Subject: Re: [PATCH 3.19.y-ckt 173/251] clocksource: exynos_mct: Avoid
> blocking calls in the cpu hotplug notifier
> 
> 2015-07-16 15:37 GMT+09:00 Duan Andy :
> > From: Kamal Mostafa  Sent: Thursday, July 16,
> > 2015 9:08 AM
> >> To: linux-kernel@vger.kernel.org; sta...@vger.kernel.org; kernel-
> >> t...@lists.ubuntu.com
> >> Cc: Kamal Mostafa; daniel.lezc...@linaro.org;
> >> kyungmin.p...@samsung.com; Damian Eppel; kg...@kernel.org; Thomas
> >> Gleixner; linux-arm- ker...@lists.infradead.org;
> >> m.szyprow...@samsung.com
> >> Subject: [PATCH 3.19.y-ckt 173/251] clocksource: exynos_mct: Avoid
> >> blocking calls in the cpu hotplug notifier
> >>
> >> 3.19.8-ckt4 -stable review patch.  If anyone has any objections,
> >> please let me know.
> >>
> >> --
> >>
> >> From: Damian Eppel 
> >>
> >> commit 56a94f13919c0db5958611b388e1581b4852f3c9 upstream.
> >>
> >> Whilst testing cpu hotplug events on kernel configured with
> >> DEBUG_PREEMPT and DEBUG_ATOMIC_SLEEP we get following BUG message,
> >> caused by calling
> >> request_irq() and free_irq() in the context of hotplug notification
> >> (which is in this case atomic context).
> >>
> >> [   40.785859] CPU1: Software reset
> >> [   40.786660] BUG: sleeping function called from invalid context at
> >> mm/slub.c:1241
> >> [   40.786668] in_atomic(): 1, irqs_disabled(): 128, pid: 0, name:
> >> swapper/1
> >> [   40.786678] Preemption disabled at:[<  (null)>]   (null)
> >> [   40.786681]
> >> [   40.786692] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 3.19.0-rc4-
> >> 00024-g7dca860 #36
> >> [   40.786698] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
> >> [   40.786728] [] (unwind_backtrace) from []
> >> (show_stack+0x10/0x14)
> >> [   40.786747] [] (show_stack) from []
> >> (dump_stack+0x70/0xbc)
> >> [   40.786767] [] (dump_stack) from []
> >> (kmem_cache_alloc+0xd8/0x170)
> >> [   40.786785] [] (kmem_cache_alloc) from []
> >> (request_threaded_irq+0x64/0x128)
> >> [   40.786804] [] (request_threaded_irq) from []
> >> (exynos4_local_timer_setup+0xc0/0x13c)
> >> [   40.786820] [] (exynos4_local_timer_setup) from
> []
> >> (exynos4_mct_cpu_notify+0x30/0xa8)
> >> [   40.786838] [] (exynos4_mct_cpu_notify) from []
> >> (notifier_call_chain+0x44/0x84)
> >> [   40.786857] [] (notifier_call_chain) from []
> >> (__cpu_notify+0x28/0x44)
> >> [   40.786873] [] (__cpu_notify) from []
> >> (secondary_start_kernel+0xec/0x150)
> >> [   40.786886] [] (secondary_start_kernel) from [<40008764>]
> >> (0x40008764)
> >>
> >> Interrupts cannot be requested/freed in the CPU_STARTING/CPU_DYING
> >> notifications which run on the hotplugged cpu with interrupts and
> >> preemption disabled.
> >>
> >> To avoid the issue, request the interrupts for all possible cpus in
> >> the boot code. The interrupts are marked NO_AUTOENABLE to avoid a
> >> racy
> >> request_irq/disable_irq() sequence. The flag prevents the
> >> request_irq() code from enabling the interrupt immediately.
> >>
> >> The interrupt is then enabled in the CPU_STARTING notifier of the
> >> hotplugged cpu and again disabled with disable_irq_nosync() in the
> >> CPU_DYING notifier.
> >>
> >> [ tglx: Massaged changelog to match the patch ]
> >>
> >> Fixes: 7114cd749a12 ("clocksource: exynos_mct: use (request/free)_irq
> >> calls for local timer registration")
> >> Reported-by: Krzysztof Kozlowski 
> >> Reviewed-by: Krzysztof Kozlowski 
> >> Tested-by: Krzysztof Kozlowski 
> >> Tested-by: Marcin Jabrzyk 
> >> Signed-off-by: Damian Eppel 
> >> Cc: m.szyprow...@samsung.com
> >> Cc: kyungmin.p...@samsung.com
> >> Cc: daniel.lezc...@linaro.org
> >> Cc: kg...@kernel.org
> >> Cc: linux-arm-ker...@lists.infradead.org
> >> Link: http://lkml.kernel.org/r/1435324984-7328-1-git-send-email-
> >> d

RE: [PATCH 3.19.y-ckt 173/251] clocksource: exynos_mct: Avoid blocking calls in the cpu hotplug notifier

[Duan Andy]

From: Kamal Mostafa  Sent: Thursday, July 16, 2015 9:08 AM
> To: linux-kernel@vger.kernel.org; sta...@vger.kernel.org; kernel-
> t...@lists.ubuntu.com
> Cc: Kamal Mostafa; daniel.lezc...@linaro.org; kyungmin.p...@samsung.com;
> Damian Eppel; kg...@kernel.org; Thomas Gleixner; linux-arm-
> ker...@lists.infradead.org; m.szyprow...@samsung.com
> Subject: [PATCH 3.19.y-ckt 173/251] clocksource: exynos_mct: Avoid
> blocking calls in the cpu hotplug notifier
> 
> 3.19.8-ckt4 -stable review patch.  If anyone has any objections, please
> let me know.
> 
> --
> 
> From: Damian Eppel 
> 
> commit 56a94f13919c0db5958611b388e1581b4852f3c9 upstream.
> 
> Whilst testing cpu hotplug events on kernel configured with DEBUG_PREEMPT
> and DEBUG_ATOMIC_SLEEP we get following BUG message, caused by calling
> request_irq() and free_irq() in the context of hotplug notification
> (which is in this case atomic context).
> 
> [   40.785859] CPU1: Software reset
> [   40.786660] BUG: sleeping function called from invalid context at
> mm/slub.c:1241
> [   40.786668] in_atomic(): 1, irqs_disabled(): 128, pid: 0, name:
> swapper/1
> [   40.786678] Preemption disabled at:[<  (null)>]   (null)
> [   40.786681]
> [   40.786692] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 3.19.0-rc4-
> 00024-g7dca860 #36
> [   40.786698] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
> [   40.786728] [] (unwind_backtrace) from []
> (show_stack+0x10/0x14)
> [   40.786747] [] (show_stack) from []
> (dump_stack+0x70/0xbc)
> [   40.786767] [] (dump_stack) from []
> (kmem_cache_alloc+0xd8/0x170)
> [   40.786785] [] (kmem_cache_alloc) from []
> (request_threaded_irq+0x64/0x128)
> [   40.786804] [] (request_threaded_irq) from []
> (exynos4_local_timer_setup+0xc0/0x13c)
> [   40.786820] [] (exynos4_local_timer_setup) from []
> (exynos4_mct_cpu_notify+0x30/0xa8)
> [   40.786838] [] (exynos4_mct_cpu_notify) from []
> (notifier_call_chain+0x44/0x84)
> [   40.786857] [] (notifier_call_chain) from []
> (__cpu_notify+0x28/0x44)
> [   40.786873] [] (__cpu_notify) from []
> (secondary_start_kernel+0xec/0x150)
> [   40.786886] [] (secondary_start_kernel) from [<40008764>]
> (0x40008764)
> 
> Interrupts cannot be requested/freed in the CPU_STARTING/CPU_DYING
> notifications which run on the hotplugged cpu with interrupts and
> preemption disabled.
> 
> To avoid the issue, request the interrupts for all possible cpus in the
> boot code. The interrupts are marked NO_AUTOENABLE to avoid a racy
> request_irq/disable_irq() sequence. The flag prevents the
> request_irq() code from enabling the interrupt immediately.
> 
> The interrupt is then enabled in the CPU_STARTING notifier of the
> hotplugged cpu and again disabled with disable_irq_nosync() in the
> CPU_DYING notifier.
> 
> [ tglx: Massaged changelog to match the patch ]
> 
> Fixes: 7114cd749a12 ("clocksource: exynos_mct: use (request/free)_irq
> calls for local timer registration")
> Reported-by: Krzysztof Kozlowski 
> Reviewed-by: Krzysztof Kozlowski 
> Tested-by: Krzysztof Kozlowski 
> Tested-by: Marcin Jabrzyk 
> Signed-off-by: Damian Eppel 
> Cc: m.szyprow...@samsung.com
> Cc: kyungmin.p...@samsung.com
> Cc: daniel.lezc...@linaro.org
> Cc: kg...@kernel.org
> Cc: linux-arm-ker...@lists.infradead.org
> Link: http://lkml.kernel.org/r/1435324984-7328-1-git-send-email-
> d.ep...@samsung.com
> Signed-off-by: Thomas Gleixner 
> Signed-off-by: Kamal Mostafa 
> ---
>  drivers/clocksource/exynos_mct.c | 43 --
> --
>  1 file changed, 30 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/clocksource/exynos_mct.c
> b/drivers/clocksource/exynos_mct.c
> index 83564c9..c844616 100644
> --- a/drivers/clocksource/exynos_mct.c
> +++ b/drivers/clocksource/exynos_mct.c
> @@ -466,15 +466,12 @@ static int exynos4_local_timer_setup(struct
> clock_event_device *evt)
>   exynos4_mct_write(TICK_BASE_CNT, mevt->base + MCT_L_TCNTB_OFFSET);
> 
>   if (mct_int_type == MCT_INT_SPI) {
> - evt->irq = mct_irqs[MCT_L0_IRQ + cpu];
> - if (request_irq(evt->irq, exynos4_mct_tick_isr,
> - IRQF_TIMER | IRQF_NOBALANCING,
> - evt->name, mevt)) {
> - pr_err("exynos-mct: cannot register IRQ %d\n",
> - evt->irq);
> +
> + if (evt->irq == -1)
>   return -EIO;
> - }
> - irq_force_affinity(mct_irqs[MCT_L0_IRQ + cpu],
> cpumask_of(cpu));
> +
> + irq_force_affinity(evt->irq, cpumask_of(cpu));
> + enable_irq(evt->irq);
>   } else {
>   enable_percpu_irq(mct_irqs[MCT_L0_IRQ], 0);

In here, why not use enable_percpu_irq(evt->irq) ?

>   }
> @@ -487,10 +484,12 @@ static int exynos4_local_timer_setup(struct
> clock_event_device *evt)  static void exynos4_local_timer_stop(struct
> clock_event_device *evt)  {
>   evt->set_mode(CLOCK_EVT_MODE_UNUSED, evt);
> - if 

RE: [PATCH 3.19.y-ckt 173/251] clocksource: exynos_mct: Avoid blocking calls in the cpu hotplug notifier

[Duan Andy]

From: Krzysztof Kozlowski k.kozlow...@samsung.com Sent: Thursday, July 16, 
2015 2:56 PM
 To: Duan Fugang-B38611
 Cc: Kamal Mostafa; linux-kernel@vger.kernel.org; sta...@vger.kernel.org;
 kernel-t...@lists.ubuntu.com; daniel.lezc...@linaro.org; Damian Eppel;
 kyungmin.p...@samsung.com; kg...@kernel.org; Thomas Gleixner; linux-arm-
 ker...@lists.infradead.org; m.szyprow...@samsung.com
 Subject: Re: [PATCH 3.19.y-ckt 173/251] clocksource: exynos_mct: Avoid
 blocking calls in the cpu hotplug notifier
 
 2015-07-16 15:37 GMT+09:00 Duan Andy fugang.d...@freescale.com:
  From: Kamal Mostafa ka...@canonical.com Sent: Thursday, July 16,
  2015 9:08 AM
  To: linux-kernel@vger.kernel.org; sta...@vger.kernel.org; kernel-
  t...@lists.ubuntu.com
  Cc: Kamal Mostafa; daniel.lezc...@linaro.org;
  kyungmin.p...@samsung.com; Damian Eppel; kg...@kernel.org; Thomas
  Gleixner; linux-arm- ker...@lists.infradead.org;
  m.szyprow...@samsung.com
  Subject: [PATCH 3.19.y-ckt 173/251] clocksource: exynos_mct: Avoid
  blocking calls in the cpu hotplug notifier
 
  3.19.8-ckt4 -stable review patch.  If anyone has any objections,
  please let me know.
 
  --
 
  From: Damian Eppel d.ep...@samsung.com
 
  commit 56a94f13919c0db5958611b388e1581b4852f3c9 upstream.
 
  Whilst testing cpu hotplug events on kernel configured with
  DEBUG_PREEMPT and DEBUG_ATOMIC_SLEEP we get following BUG message,
  caused by calling
  request_irq() and free_irq() in the context of hotplug notification
  (which is in this case atomic context).
 
  [   40.785859] CPU1: Software reset
  [   40.786660] BUG: sleeping function called from invalid context at
  mm/slub.c:1241
  [   40.786668] in_atomic(): 1, irqs_disabled(): 128, pid: 0, name:
  swapper/1
  [   40.786678] Preemption disabled at:[  (null)]   (null)
  [   40.786681]
  [   40.786692] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 3.19.0-rc4-
  00024-g7dca860 #36
  [   40.786698] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
  [   40.786728] [c0014a00] (unwind_backtrace) from [c0011980]
  (show_stack+0x10/0x14)
  [   40.786747] [c0011980] (show_stack) from [c0449ba0]
  (dump_stack+0x70/0xbc)
  [   40.786767] [c0449ba0] (dump_stack) from [c00c6124]
  (kmem_cache_alloc+0xd8/0x170)
  [   40.786785] [c00c6124] (kmem_cache_alloc) from [c005d6f8]
  (request_threaded_irq+0x64/0x128)
  [   40.786804] [c005d6f8] (request_threaded_irq) from [c0350b8c]
  (exynos4_local_timer_setup+0xc0/0x13c)
  [   40.786820] [c0350b8c] (exynos4_local_timer_setup) from
 [c0350ca8]
  (exynos4_mct_cpu_notify+0x30/0xa8)
  [   40.786838] [c0350ca8] (exynos4_mct_cpu_notify) from [c003b330]
  (notifier_call_chain+0x44/0x84)
  [   40.786857] [c003b330] (notifier_call_chain) from [c0022fd4]
  (__cpu_notify+0x28/0x44)
  [   40.786873] [c0022fd4] (__cpu_notify) from [c0013714]
  (secondary_start_kernel+0xec/0x150)
  [   40.786886] [c0013714] (secondary_start_kernel) from [40008764]
  (0x40008764)
 
  Interrupts cannot be requested/freed in the CPU_STARTING/CPU_DYING
  notifications which run on the hotplugged cpu with interrupts and
  preemption disabled.
 
  To avoid the issue, request the interrupts for all possible cpus in
  the boot code. The interrupts are marked NO_AUTOENABLE to avoid a
  racy
  request_irq/disable_irq() sequence. The flag prevents the
  request_irq() code from enabling the interrupt immediately.
 
  The interrupt is then enabled in the CPU_STARTING notifier of the
  hotplugged cpu and again disabled with disable_irq_nosync() in the
  CPU_DYING notifier.
 
  [ tglx: Massaged changelog to match the patch ]
 
  Fixes: 7114cd749a12 (clocksource: exynos_mct: use (request/free)_irq
  calls for local timer registration)
  Reported-by: Krzysztof Kozlowski k.kozlow...@samsung.com
  Reviewed-by: Krzysztof Kozlowski k.kozlow...@samsung.com
  Tested-by: Krzysztof Kozlowski k.kozlow...@samsung.com
  Tested-by: Marcin Jabrzyk m.jabr...@samsung.com
  Signed-off-by: Damian Eppel d.ep...@samsung.com
  Cc: m.szyprow...@samsung.com
  Cc: kyungmin.p...@samsung.com
  Cc: daniel.lezc...@linaro.org
  Cc: kg...@kernel.org
  Cc: linux-arm-ker...@lists.infradead.org
  Link: http://lkml.kernel.org/r/1435324984-7328-1-git-send-email-
  d.ep...@samsung.com
  Signed-off-by: Thomas Gleixner t...@linutronix.de
  Signed-off-by: Kamal Mostafa ka...@canonical.com
  ---
   drivers/clocksource/exynos_mct.c | 43
  --
  --
   1 file changed, 30 insertions(+), 13 deletions(-)
 
  diff --git a/drivers/clocksource/exynos_mct.c
  b/drivers/clocksource/exynos_mct.c
  index 83564c9..c844616 100644
  --- a/drivers/clocksource/exynos_mct.c
  +++ b/drivers/clocksource/exynos_mct.c
  @@ -466,15 +466,12 @@ static int exynos4_local_timer_setup(struct
  clock_event_device *evt)
exynos4_mct_write(TICK_BASE_CNT, mevt-base +
  MCT_L_TCNTB_OFFSET);
 
if (mct_int_type == MCT_INT_SPI) {
  - evt-irq = mct_irqs[MCT_L0_IRQ + cpu];
  - if (request_irq(evt-irq

RE: [PATCH 3.19.y-ckt 173/251] clocksource: exynos_mct: Avoid blocking calls in the cpu hotplug notifier

[Duan Andy]

From: Kamal Mostafa ka...@canonical.com Sent: Thursday, July 16, 2015 9:08 AM
 To: linux-kernel@vger.kernel.org; sta...@vger.kernel.org; kernel-
 t...@lists.ubuntu.com
 Cc: Kamal Mostafa; daniel.lezc...@linaro.org; kyungmin.p...@samsung.com;
 Damian Eppel; kg...@kernel.org; Thomas Gleixner; linux-arm-
 ker...@lists.infradead.org; m.szyprow...@samsung.com
 Subject: [PATCH 3.19.y-ckt 173/251] clocksource: exynos_mct: Avoid
 blocking calls in the cpu hotplug notifier
 
 3.19.8-ckt4 -stable review patch.  If anyone has any objections, please
 let me know.
 
 --
 
 From: Damian Eppel d.ep...@samsung.com
 
 commit 56a94f13919c0db5958611b388e1581b4852f3c9 upstream.
 
 Whilst testing cpu hotplug events on kernel configured with DEBUG_PREEMPT
 and DEBUG_ATOMIC_SLEEP we get following BUG message, caused by calling
 request_irq() and free_irq() in the context of hotplug notification
 (which is in this case atomic context).
 
 [   40.785859] CPU1: Software reset
 [   40.786660] BUG: sleeping function called from invalid context at
 mm/slub.c:1241
 [   40.786668] in_atomic(): 1, irqs_disabled(): 128, pid: 0, name:
 swapper/1
 [   40.786678] Preemption disabled at:[  (null)]   (null)
 [   40.786681]
 [   40.786692] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 3.19.0-rc4-
 00024-g7dca860 #36
 [   40.786698] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
 [   40.786728] [c0014a00] (unwind_backtrace) from [c0011980]
 (show_stack+0x10/0x14)
 [   40.786747] [c0011980] (show_stack) from [c0449ba0]
 (dump_stack+0x70/0xbc)
 [   40.786767] [c0449ba0] (dump_stack) from [c00c6124]
 (kmem_cache_alloc+0xd8/0x170)
 [   40.786785] [c00c6124] (kmem_cache_alloc) from [c005d6f8]
 (request_threaded_irq+0x64/0x128)
 [   40.786804] [c005d6f8] (request_threaded_irq) from [c0350b8c]
 (exynos4_local_timer_setup+0xc0/0x13c)
 [   40.786820] [c0350b8c] (exynos4_local_timer_setup) from [c0350ca8]
 (exynos4_mct_cpu_notify+0x30/0xa8)
 [   40.786838] [c0350ca8] (exynos4_mct_cpu_notify) from [c003b330]
 (notifier_call_chain+0x44/0x84)
 [   40.786857] [c003b330] (notifier_call_chain) from [c0022fd4]
 (__cpu_notify+0x28/0x44)
 [   40.786873] [c0022fd4] (__cpu_notify) from [c0013714]
 (secondary_start_kernel+0xec/0x150)
 [   40.786886] [c0013714] (secondary_start_kernel) from [40008764]
 (0x40008764)
 
 Interrupts cannot be requested/freed in the CPU_STARTING/CPU_DYING
 notifications which run on the hotplugged cpu with interrupts and
 preemption disabled.
 
 To avoid the issue, request the interrupts for all possible cpus in the
 boot code. The interrupts are marked NO_AUTOENABLE to avoid a racy
 request_irq/disable_irq() sequence. The flag prevents the
 request_irq() code from enabling the interrupt immediately.
 
 The interrupt is then enabled in the CPU_STARTING notifier of the
 hotplugged cpu and again disabled with disable_irq_nosync() in the
 CPU_DYING notifier.
 
 [ tglx: Massaged changelog to match the patch ]
 
 Fixes: 7114cd749a12 (clocksource: exynos_mct: use (request/free)_irq
 calls for local timer registration)
 Reported-by: Krzysztof Kozlowski k.kozlow...@samsung.com
 Reviewed-by: Krzysztof Kozlowski k.kozlow...@samsung.com
 Tested-by: Krzysztof Kozlowski k.kozlow...@samsung.com
 Tested-by: Marcin Jabrzyk m.jabr...@samsung.com
 Signed-off-by: Damian Eppel d.ep...@samsung.com
 Cc: m.szyprow...@samsung.com
 Cc: kyungmin.p...@samsung.com
 Cc: daniel.lezc...@linaro.org
 Cc: kg...@kernel.org
 Cc: linux-arm-ker...@lists.infradead.org
 Link: http://lkml.kernel.org/r/1435324984-7328-1-git-send-email-
 d.ep...@samsung.com
 Signed-off-by: Thomas Gleixner t...@linutronix.de
 Signed-off-by: Kamal Mostafa ka...@canonical.com
 ---
  drivers/clocksource/exynos_mct.c | 43 --
 --
  1 file changed, 30 insertions(+), 13 deletions(-)
 
 diff --git a/drivers/clocksource/exynos_mct.c
 b/drivers/clocksource/exynos_mct.c
 index 83564c9..c844616 100644
 --- a/drivers/clocksource/exynos_mct.c
 +++ b/drivers/clocksource/exynos_mct.c
 @@ -466,15 +466,12 @@ static int exynos4_local_timer_setup(struct
 clock_event_device *evt)
   exynos4_mct_write(TICK_BASE_CNT, mevt-base + MCT_L_TCNTB_OFFSET);
 
   if (mct_int_type == MCT_INT_SPI) {
 - evt-irq = mct_irqs[MCT_L0_IRQ + cpu];
 - if (request_irq(evt-irq, exynos4_mct_tick_isr,
 - IRQF_TIMER | IRQF_NOBALANCING,
 - evt-name, mevt)) {
 - pr_err(exynos-mct: cannot register IRQ %d\n,
 - evt-irq);
 +
 + if (evt-irq == -1)
   return -EIO;
 - }
 - irq_force_affinity(mct_irqs[MCT_L0_IRQ + cpu],
 cpumask_of(cpu));
 +
 + irq_force_affinity(evt-irq, cpumask_of(cpu));
 + enable_irq(evt-irq);
   } else {
   enable_percpu_irq(mct_irqs[MCT_L0_IRQ], 0);

In here, why not use enable_percpu_irq(evt-irq) ?

   }
 @@ -487,10 +484,12 @@ static int 

RE: [PATCH 3.19.y-ckt 173/251] clocksource: exynos_mct: Avoid blocking calls in the cpu hotplug notifier

[Duan Andy]

From: Krzysztof Kozlowski k.kozlow...@samsung.com Sent: Thursday, July 16, 
2015 5:52 PM
 To: Duan Fugang-B38611
 Cc: Krzysztof Kozlowski; Kamal Mostafa; daniel.lezc...@linaro.org; linux-
 ker...@vger.kernel.org; sta...@vger.kernel.org; Damian Eppel;
 kg...@kernel.org; kyungmin.p...@samsung.com; Thomas Gleixner;
 m.szyprow...@samsung.com; linux-arm-ker...@lists.infradead.org; kernel-
 t...@lists.ubuntu.com
 Subject: Re: [PATCH 3.19.y-ckt 173/251] clocksource: exynos_mct: Avoid
 blocking calls in the cpu hotplug notifier
 
 2015-07-16 17:14 GMT+09:00 Duan Andy fugang.d...@freescale.com:
  From: Krzysztof Kozlowski k.kozlow...@samsung.com Sent: Thursday,
  July 16, 2015 2:56 PM
  To: Duan Fugang-B38611
  Cc: Kamal Mostafa; linux-kernel@vger.kernel.org;
  sta...@vger.kernel.org; kernel-t...@lists.ubuntu.com;
  daniel.lezc...@linaro.org; Damian Eppel; kyungmin.p...@samsung.com;
  kg...@kernel.org; Thomas Gleixner; linux-arm-
  ker...@lists.infradead.org; m.szyprow...@samsung.com
  Subject: Re: [PATCH 3.19.y-ckt 173/251] clocksource: exynos_mct:
  Avoid blocking calls in the cpu hotplug notifier
 
  2015-07-16 15:37 GMT+09:00 Duan Andy fugang.d...@freescale.com:
   From: Kamal Mostafa ka...@canonical.com Sent: Thursday, July 16,
   2015 9:08 AM
   To: linux-kernel@vger.kernel.org; sta...@vger.kernel.org; kernel-
   t...@lists.ubuntu.com
   Cc: Kamal Mostafa; daniel.lezc...@linaro.org;
   kyungmin.p...@samsung.com; Damian Eppel; kg...@kernel.org; Thomas
   Gleixner; linux-arm- ker...@lists.infradead.org;
   m.szyprow...@samsung.com
   Subject: [PATCH 3.19.y-ckt 173/251] clocksource: exynos_mct: Avoid
   blocking calls in the cpu hotplug notifier
  
   3.19.8-ckt4 -stable review patch.  If anyone has any objections,
   please let me know.
  
   --
  
   From: Damian Eppel d.ep...@samsung.com
  
   commit 56a94f13919c0db5958611b388e1581b4852f3c9 upstream.
  
   Whilst testing cpu hotplug events on kernel configured with
   DEBUG_PREEMPT and DEBUG_ATOMIC_SLEEP we get following BUG message,
   caused by calling
   request_irq() and free_irq() in the context of hotplug
   notification (which is in this case atomic context).
  
   [   40.785859] CPU1: Software reset
   [   40.786660] BUG: sleeping function called from invalid context
 at
   mm/slub.c:1241
   [   40.786668] in_atomic(): 1, irqs_disabled(): 128, pid: 0, name:
   swapper/1
   [   40.786678] Preemption disabled at:[  (null)]   (null)
   [   40.786681]
   [   40.786692] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 3.19.0-
 rc4-
   00024-g7dca860 #36
   [   40.786698] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
   [   40.786728] [c0014a00] (unwind_backtrace) from [c0011980]
   (show_stack+0x10/0x14)
   [   40.786747] [c0011980] (show_stack) from [c0449ba0]
   (dump_stack+0x70/0xbc)
   [   40.786767] [c0449ba0] (dump_stack) from [c00c6124]
   (kmem_cache_alloc+0xd8/0x170)
   [   40.786785] [c00c6124] (kmem_cache_alloc) from [c005d6f8]
   (request_threaded_irq+0x64/0x128)
   [   40.786804] [c005d6f8] (request_threaded_irq) from [c0350b8c]
   (exynos4_local_timer_setup+0xc0/0x13c)
   [   40.786820] [c0350b8c] (exynos4_local_timer_setup) from
  [c0350ca8]
   (exynos4_mct_cpu_notify+0x30/0xa8)
   [   40.786838] [c0350ca8] (exynos4_mct_cpu_notify) from
 [c003b330]
   (notifier_call_chain+0x44/0x84)
   [   40.786857] [c003b330] (notifier_call_chain) from [c0022fd4]
   (__cpu_notify+0x28/0x44)
   [   40.786873] [c0022fd4] (__cpu_notify) from [c0013714]
   (secondary_start_kernel+0xec/0x150)
   [   40.786886] [c0013714] (secondary_start_kernel) from
 [40008764]
   (0x40008764)
  
   Interrupts cannot be requested/freed in the CPU_STARTING/CPU_DYING
   notifications which run on the hotplugged cpu with interrupts and
   preemption disabled.
  
   To avoid the issue, request the interrupts for all possible cpus
   in the boot code. The interrupts are marked NO_AUTOENABLE to avoid
   a racy
   request_irq/disable_irq() sequence. The flag prevents the
   request_irq() code from enabling the interrupt immediately.
  
   The interrupt is then enabled in the CPU_STARTING notifier of the
   hotplugged cpu and again disabled with disable_irq_nosync() in the
   CPU_DYING notifier.
  
   [ tglx: Massaged changelog to match the patch ]
  
   Fixes: 7114cd749a12 (clocksource: exynos_mct: use
   (request/free)_irq calls for local timer registration)
   Reported-by: Krzysztof Kozlowski k.kozlow...@samsung.com
   Reviewed-by: Krzysztof Kozlowski k.kozlow...@samsung.com
   Tested-by: Krzysztof Kozlowski k.kozlow...@samsung.com
   Tested-by: Marcin Jabrzyk m.jabr...@samsung.com
   Signed-off-by: Damian Eppel d.ep...@samsung.com
   Cc: m.szyprow...@samsung.com
   Cc: kyungmin.p...@samsung.com
   Cc: daniel.lezc...@linaro.org
   Cc: kg...@kernel.org
   Cc: linux-arm-ker...@lists.infradead.org
   Link: http://lkml.kernel.org/r/1435324984-7328-1-git-send-email-
   d.ep...@samsung.com
   Signed-off-by: Thomas Gleixner t...@linutronix.de
   Signed-off-by: Kamal

RE: [PATCH v2 0/2] Implement sample time consideration for Vybrid's ADC

[Duan Andy]

From: Sanchayan Maity  Sent: Wednesday, June 24, 2015 
4:34 PM
> To: ji...@kernel.org
> Cc: shawn@linaro.org; ker...@pengutronix.de; robh...@kernel.org;
> pawel.m...@arm.com; mark.rutl...@arm.com; ijc+devicet...@hellion.org.uk;
> ga...@codeaurora.org; Duan Fugang-B38611; devicet...@vger.kernel.org;
> linux-...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; linux-
> ker...@vger.kernel.org; ste...@agner.ch; Sanchayan Maity
> Subject: [PATCH v2 0/2] Implement sample time consideration for Vybrid's
> ADC
> 
> Hello,
> 
> This patchset adds a dt binding for specifying sample time for the vybrid
> adc driver and takes this into account for sampling frequency calculation
> and related configuration in the driver.
> 
> The patchset is based on top of Stefan's patches here
> http://lkml.iu.edu/hypermail/linux/kernel/1505.3/02043.html
> 
> which got recently applied. Tested with shawn's for-next branch.
> 
> Changes since v1:
> 
> Change from a vendor specific fsl,min-sample-time to non vendor specific
> min-sample-time.
> 
> Version 1 of the patchset can be found here
> http://lkml.iu.edu/hypermail/linux/kernel/1506.1/00026.html
> 
> - Sanchayan.
> 
> Sanchayan Maity (2):
>   iio: adc: Determine sampling frequencies by using minimum sample time
>   ARM: dts: vfxxx: Add property for minimum sample time
> 
>  .../devicetree/bindings/iio/adc/vf610-adc.txt  |  6 ++
>  arch/arm/boot/dts/vfxxx.dtsi   |  2 +
>  drivers/iio/adc/vf610_adc.c| 74
> --
>  3 files changed, 78 insertions(+), 4 deletions(-)

The patch set looks fine. Thanks Sanchayan add this new interface for user that 
is more options for user case.

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

RE: [PATCH v2 0/2] Implement sample time consideration for Vybrid's ADC

[Duan Andy]

From: Sanchayan Maity maitysancha...@gmail.com Sent: Wednesday, June 24, 2015 
4:34 PM
 To: ji...@kernel.org
 Cc: shawn@linaro.org; ker...@pengutronix.de; robh...@kernel.org;
 pawel.m...@arm.com; mark.rutl...@arm.com; ijc+devicet...@hellion.org.uk;
 ga...@codeaurora.org; Duan Fugang-B38611; devicet...@vger.kernel.org;
 linux-...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; linux-
 ker...@vger.kernel.org; ste...@agner.ch; Sanchayan Maity
 Subject: [PATCH v2 0/2] Implement sample time consideration for Vybrid's
 ADC
 
 Hello,
 
 This patchset adds a dt binding for specifying sample time for the vybrid
 adc driver and takes this into account for sampling frequency calculation
 and related configuration in the driver.
 
 The patchset is based on top of Stefan's patches here
 http://lkml.iu.edu/hypermail/linux/kernel/1505.3/02043.html
 
 which got recently applied. Tested with shawn's for-next branch.
 
 Changes since v1:
 
 Change from a vendor specific fsl,min-sample-time to non vendor specific
 min-sample-time.
 
 Version 1 of the patchset can be found here
 http://lkml.iu.edu/hypermail/linux/kernel/1506.1/00026.html
 
 - Sanchayan.
 
 Sanchayan Maity (2):
   iio: adc: Determine sampling frequencies by using minimum sample time
   ARM: dts: vfxxx: Add property for minimum sample time
 
  .../devicetree/bindings/iio/adc/vf610-adc.txt  |  6 ++
  arch/arm/boot/dts/vfxxx.dtsi   |  2 +
  drivers/iio/adc/vf610_adc.c| 74
 --
  3 files changed, 78 insertions(+), 4 deletions(-)

The patch set looks fine. Thanks Sanchayan add this new interface for user that 
is more options for user case.

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