Re: [PATCH net-next] nxp: fix trivial comment typo
Hello Andrea, On 11/14/2018 08:47 PM, Andrea Claudi wrote: > s/rxfliterctrl/rxfilterctrl > > Signed-off-by: Andrea Claudi thank you for the patch, but let me ask you to change the subject line by adding the expected prefixes 'net: lpc_eth: fix trivial comment typo'. Also it would be nice to see a simple non-empty commit description. -- Best wishes, Vladimir
[PATCH] net: ethernet: lpc_eth: add device and device node local variables
Trivial non-functional change added to simplify getting multiple
references to device pointer in lpc_eth_drv_probe().
Signed-off-by: Vladimir Zapolskiy
---
NB, to avoid a merge conflict the change should be applied
after https://marc.info/?l=linux-netdev&m=153990402012273
drivers/net/ethernet/nxp/lpc_eth.c | 40 --
1 file changed, 21 insertions(+), 19 deletions(-)
diff --git a/drivers/net/ethernet/nxp/lpc_eth.c
b/drivers/net/ethernet/nxp/lpc_eth.c
index e275d64007af..ecc7f464c238 100644
--- a/drivers/net/ethernet/nxp/lpc_eth.c
+++ b/drivers/net/ethernet/nxp/lpc_eth.c
@@ -1242,17 +1242,19 @@ static const struct net_device_ops lpc_netdev_ops = {
static int lpc_eth_drv_probe(struct platform_device *pdev)
{
- struct resource *res;
- struct net_device *ndev;
+ struct device *dev = &pdev->dev;
+ struct device_node *np = dev->of_node;
struct netdata_local *pldat;
+ struct net_device *ndev;
dma_addr_t dma_handle;
+ struct resource *res;
int irq, ret;
u32 tmp;
/* Setup network interface for RMII or MII mode */
tmp = __raw_readl(LPC32XX_CLKPWR_MACCLK_CTRL);
tmp &= ~LPC32XX_CLKPWR_MACCTRL_PINS_MSK;
- if (lpc_phy_interface_mode(&pdev->dev) == PHY_INTERFACE_MODE_MII)
+ if (lpc_phy_interface_mode(dev) == PHY_INTERFACE_MODE_MII)
tmp |= LPC32XX_CLKPWR_MACCTRL_USE_MII_PINS;
else
tmp |= LPC32XX_CLKPWR_MACCTRL_USE_RMII_PINS;
@@ -1262,7 +1264,7 @@ static int lpc_eth_drv_probe(struct platform_device *pdev)
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
irq = platform_get_irq(pdev, 0);
if (!res || irq < 0) {
- dev_err(&pdev->dev, "error getting resources.\n");
+ dev_err(dev, "error getting resources.\n");
ret = -ENXIO;
goto err_exit;
}
@@ -1270,12 +1272,12 @@ static int lpc_eth_drv_probe(struct platform_device
*pdev)
/* Allocate net driver data structure */
ndev = alloc_etherdev(sizeof(struct netdata_local));
if (!ndev) {
- dev_err(&pdev->dev, "could not allocate device.\n");
+ dev_err(dev, "could not allocate device.\n");
ret = -ENOMEM;
goto err_exit;
}
- SET_NETDEV_DEV(ndev, &pdev->dev);
+ SET_NETDEV_DEV(ndev, dev);
pldat = netdev_priv(ndev);
pldat->pdev = pdev;
@@ -1287,9 +1289,9 @@ static int lpc_eth_drv_probe(struct platform_device *pdev)
ndev->irq = irq;
/* Get clock for the device */
- pldat->clk = clk_get(&pdev->dev, NULL);
+ pldat->clk = clk_get(dev, NULL);
if (IS_ERR(pldat->clk)) {
- dev_err(&pdev->dev, "error getting clock.\n");
+ dev_err(dev, "error getting clock.\n");
ret = PTR_ERR(pldat->clk);
goto err_out_free_dev;
}
@@ -1302,14 +1304,14 @@ static int lpc_eth_drv_probe(struct platform_device
*pdev)
/* Map IO space */
pldat->net_base = ioremap(res->start, resource_size(res));
if (!pldat->net_base) {
- dev_err(&pdev->dev, "failed to map registers\n");
+ dev_err(dev, "failed to map registers\n");
ret = -ENOMEM;
goto err_out_disable_clocks;
}
ret = request_irq(ndev->irq, __lpc_eth_interrupt, 0,
ndev->name, ndev);
if (ret) {
- dev_err(&pdev->dev, "error requesting interrupt.\n");
+ dev_err(dev, "error requesting interrupt.\n");
goto err_out_iounmap;
}
@@ -1323,7 +1325,7 @@ static int lpc_eth_drv_probe(struct platform_device *pdev)
sizeof(struct txrx_desc_t) + sizeof(struct rx_status_t));
pldat->dma_buff_base_v = 0;
- if (use_iram_for_net(&pldat->pdev->dev)) {
+ if (use_iram_for_net(dev)) {
dma_handle = LPC32XX_IRAM_BASE;
if (pldat->dma_buff_size <= lpc32xx_return_iram_size())
pldat->dma_buff_base_v =
@@ -1334,7 +1336,7 @@ static int lpc_eth_drv_probe(struct platform_device *pdev)
}
if (pldat->dma_buff_base_v == 0) {
- ret = dma_coerce_mask_and_coherent(&pdev->dev,
DMA_BIT_MASK(32));
+ ret = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(32));
if (ret)
goto err_out_free_irq;
@@ -1343,7 +1345,7 @@ static int lpc_eth_drv_probe(struct platform_device *pdev)
/* Allocate a chunk of memory for the DMA ethernet buffers
and descriptors */
pldat->dma_buff_base_
[PATCH] net: ethernet: lpc_eth: remove unused local variable
A trivial change which removes an unused local variable, the issue is reported as a compile time warning: drivers/net/ethernet/nxp/lpc_eth.c: In function 'lpc_eth_drv_probe': drivers/net/ethernet/nxp/lpc_eth.c:1250:21: warning: variable 'phydev' set but not used [-Wunused-but-set-variable] struct phy_device *phydev; ^~~~~~ Signed-off-by: Vladimir Zapolskiy --- drivers/net/ethernet/nxp/lpc_eth.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/net/ethernet/nxp/lpc_eth.c b/drivers/net/ethernet/nxp/lpc_eth.c index 922b5b5b5c01..e275d64007af 100644 --- a/drivers/net/ethernet/nxp/lpc_eth.c +++ b/drivers/net/ethernet/nxp/lpc_eth.c @@ -1245,7 +1245,6 @@ static int lpc_eth_drv_probe(struct platform_device *pdev) struct resource *res; struct net_device *ndev; struct netdata_local *pldat; - struct phy_device *phydev; dma_addr_t dma_handle; int irq, ret; u32 tmp; @@ -1411,8 +1410,6 @@ static int lpc_eth_drv_probe(struct platform_device *pdev) netdev_info(ndev, "LPC mac at 0x%08x irq %d\n", res->start, ndev->irq); - phydev = ndev->phydev; - device_init_wakeup(&pdev->dev, 1); device_set_wakeup_enable(&pdev->dev, 0); -- 2.17.1
[PATCH] net: ethernet: lpc_eth: remove CONFIG_OF guard from the driver
The MAC controller device is available on NXP LPC32xx platform only,
and the LPC32xx platform supports OF builds only, so additional
checks in the device driver are not needed.
Signed-off-by: Vladimir Zapolskiy
---
drivers/net/ethernet/nxp/lpc_eth.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/nxp/lpc_eth.c
b/drivers/net/ethernet/nxp/lpc_eth.c
index ed02e8e18f25..922b5b5b5c01 100644
--- a/drivers/net/ethernet/nxp/lpc_eth.c
+++ b/drivers/net/ethernet/nxp/lpc_eth.c
@@ -1518,13 +1518,11 @@ static int lpc_eth_drv_resume(struct platform_device
*pdev)
}
#endif
-#ifdef CONFIG_OF
static const struct of_device_id lpc_eth_match[] = {
{ .compatible = "nxp,lpc-eth" },
{ }
};
MODULE_DEVICE_TABLE(of, lpc_eth_match);
-#endif
static struct platform_driver lpc_eth_driver = {
.probe = lpc_eth_drv_probe,
@@ -1535,7 +1533,7 @@ static struct platform_driver lpc_eth_driver = {
#endif
.driver = {
.name = MODNAME,
- .of_match_table = of_match_ptr(lpc_eth_match),
+ .of_match_table = lpc_eth_match,
},
};
--
2.17.1
[PATCH] net: ethernet: lpc_eth: clean up the list of included headers
The change removes all unnecessary included headers from the driver source code, the remaining list is sorted in alphabetical order. Signed-off-by: Vladimir Zapolskiy --- drivers/net/ethernet/nxp/lpc_eth.c | 28 ++-- 1 file changed, 6 insertions(+), 22 deletions(-) diff --git a/drivers/net/ethernet/nxp/lpc_eth.c b/drivers/net/ethernet/nxp/lpc_eth.c index 08381ef8bdb4..ed02e8e18f25 100644 --- a/drivers/net/ethernet/nxp/lpc_eth.c +++ b/drivers/net/ethernet/nxp/lpc_eth.c @@ -19,34 +19,18 @@ #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include #include -#include -#include +#include #include -#include -#include -#include -#include +#include #include -#include +#include +#include +#include -#include #include -#include #include +#include #define MODNAME "lpc-eth" #define DRV_VERSION "1.00" -- 2.17.1
Re: [PATCH 3/6] ravb: remove custom .set_link_ksettings from ethtool ops
Hello Sergei,
On 05/26/2018 10:50 PM, Sergei Shtylyov wrote:
> On 05/24/2018 02:11 PM, Vladimir Zapolskiy wrote:
>
>> The change replaces a custom implementation of .set_link_ksettings
>> callback with a shared phy_ethtool_set_link_ksettings(), this fixes
>> sleep in atomic context bug, which is encountered every time when link
>> settings are changed by ethtool.
>
>Seeing it now...
>
>> Now duplex mode setting is enforced in ravb_adjust_link() only, also
>> now TX/RX is disabled when link is put down or modifications to E-MAC
>> registers ECMR and GECMR are expected for both cases of checked and
>> ignored link status pin state from E-MAC interrupt handler.
>>
>> Signed-off-by: Vladimir Zapolskiy
>> ---
>> drivers/net/ethernet/renesas/ravb_main.c | 58
>> +---
>> 1 file changed, 15 insertions(+), 43 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/renesas/ravb_main.c
>> b/drivers/net/ethernet/renesas/ravb_main.c
>> index 3d91caa44176..0d811c02ff34 100644
>> --- a/drivers/net/ethernet/renesas/ravb_main.c
>> +++ b/drivers/net/ethernet/renesas/ravb_main.c
>> @@ -980,6 +980,13 @@ static void ravb_adjust_link(struct net_device *ndev)
>> struct ravb_private *priv = netdev_priv(ndev);
>> struct phy_device *phydev = ndev->phydev;
>> bool new_state = false;
>> +unsigned long flags;
>> +
>> +spin_lock_irqsave(&priv->lock, flags);
>> +
>> +/* Disable TX and RX right over here, if E-MAC change is ignored */
>> +if (priv->no_avb_link)
>> +ravb_rcv_snd_disable(ndev);
>>
>> if (phydev->link) {
>> if (phydev->duplex != priv->duplex) {
>> @@ -997,18 +1004,21 @@ static void ravb_adjust_link(struct net_device *ndev)
>> ravb_modify(ndev, ECMR, ECMR_TXF, 0);
>> new_state = true;
>> priv->link = phydev->link;
>> -if (priv->no_avb_link)
>> -ravb_rcv_snd_enable(ndev);
>> }
>> } else if (priv->link) {
>> new_state = true;
>> priv->link = 0;
>> priv->speed = 0;
>> priv->duplex = -1;
>> -if (priv->no_avb_link)
>> -ravb_rcv_snd_disable(ndev);
>> }
>>
>> +/* Enable TX and RX right over here, if E-MAC change is ignored */
>> +if (priv->no_avb_link && phydev->link)
>> +ravb_rcv_snd_enable(ndev);
>> +
>> +mmiowb();
>> +spin_unlock_irqrestore(&priv->lock, flags);
>> +
>
>I like this part. :-)
>
A weight off my mind :) And I hope that this change will remain the less
questionable one, other ones from the series are trivial.
Anyway I hope it is understandable that this part of the change can not
be simply extracted from the rest one below, otherwise there'll be bugs of
another type intorduced.
>> if (new_state && netif_msg_link(priv))
>> phy_print_status(phydev);
>> }
>> @@ -1096,44 +1106,6 @@ static int ravb_phy_start(struct net_device *ndev)
>> return 0;
>> }
>>
>> -static int ravb_set_link_ksettings(struct net_device *ndev,
>> - const struct ethtool_link_ksettings *cmd)
>> -{
>> -struct ravb_private *priv = netdev_priv(ndev);
>> -unsigned long flags;
>> -int error;
>> -
>> -if (!ndev->phydev)
>> -return -ENODEV;
>> -
>> -spin_lock_irqsave(&priv->lock, flags);
>> -
>> -/* Disable TX and RX */
>> -ravb_rcv_snd_disable(ndev);
>> -
>> -error = phy_ethtool_ksettings_set(ndev->phydev, cmd);
>> -if (error)
>> -goto error_exit;
>> -
>> -if (cmd->base.duplex == DUPLEX_FULL)
>> -priv->duplex = 1;
>> -else
>> -priv->duplex = 0;
>> -
>> -ravb_set_duplex(ndev);
>> -
>> -error_exit:
>> -mdelay(1);
>> -
>> -/* Enable TX and RX */
>> -ravb_rcv_snd_enable(ndev);
>> -
>> -mmiowb();
>> -spin_unlock_irqrestore(&priv->lock, flags);
>> -
>> -return error;
>> -}
>> -
>
>But this part is clearly lumping it all together...
Please elaborate.
>
> [...]
>> @@ -1357,7 +1329,7 @@ static const struct ethtool_ops ravb_ethtool_ops = {
>> .set_ringparam = ravb
Re: [PATCH 0/6] ravb/sh_eth: fix sleep in atomic by reusing shared ethtool handlers
Hello Sergei, On 05/24/2018 08:24 PM, Sergei Shtylyov wrote: > On 05/24/2018 07:40 PM, Sergei Shtylyov wrote: > >>> For ages trivial changes to RAVB and SuperH ethernet links by means of >>> standard 'ethtool' trigger a 'sleeping function called from invalid >>> context' bug, to visualize it on r8a7795 ULCB: >>> >>> % ethtool -r eth0 >>> BUG: sleeping function called from invalid context at >>> kernel/locking/mutex.c:747 >>> in_atomic(): 1, irqs_disabled(): 128, pid: 554, name: ethtool >>> INFO: lockdep is turned off. >>> irq event stamp: 0 >>> hardirqs last enabled at (0): [<>] (null) >>> hardirqs last disabled at (0): [] >>> copy_process.isra.7.part.8+0x2cc/0x1918 >>> softirqs last enabled at (0): [] >>> copy_process.isra.7.part.8+0x2cc/0x1918 >>> softirqs last disabled at (0): [<>] (null) >>> CPU: 5 PID: 554 Comm: ethtool Not tainted 4.17.0-rc4-arm64-renesas+ #33 >>> Hardware name: Renesas H3ULCB board based on r8a7795 ES2.0+ (DT) >>> Call trace: >>>dump_backtrace+0x0/0x198 >>>show_stack+0x24/0x30 >>>dump_stack+0xb8/0xf4 >>>___might_sleep+0x1c8/0x1f8 >>>__might_sleep+0x58/0x90 >>>__mutex_lock+0x50/0x890 >>>mutex_lock_nested+0x3c/0x50 >>>phy_start_aneg_priv+0x38/0x180 >>>phy_start_aneg+0x24/0x30 >>>ravb_nway_reset+0x3c/0x68 >>>dev_ethtool+0x3dc/0x2338 >>>dev_ioctl+0x19c/0x490 >>>sock_do_ioctl+0xe0/0x238 >>>sock_ioctl+0x254/0x460 >>>do_vfs_ioctl+0xb0/0x918 >>>ksys_ioctl+0x50/0x80 >>>sys_ioctl+0x34/0x48 >>>__sys_trace_return+0x0/0x4 >>> >>> The root cause is that an attempt to modify ECMR and GECMR registers >>> only when RX/TX function is disabled was too overcomplicated in its >>> original implementation, also processing of an optional Link Change >>> interrupt added even more complexity, as a result the implementation >>> was error prone. >>> >>> The new locking scheme is confirmed to be correct by dumping driver >>> specific and generic PHY framework function calls with aid of ftrace >>> while running more or less advanced tests. >>> >>> Please note that sh_eth patches from the series were built-tested only. >>> >>> On purpose I do not add Fixes tags, the reused PHY handlers were added >>> way later than the fixed problems were firstly found in the drivers. >> >>I think you went one step too far with these fixes. On the first glance, >> the real fixes are to remove grabbing/releasing the spinlock for the duration >> of the phylib calls. Am I right? If so, making use of the new phylib APIs >> would be a further enhancement, it's not needed for fixing the splats per >> se... > >Note that I hadn't looked at the patches #3/#6 at the time of writing this; > those seem to be more complicated than the rest. Right, the simplistic approach of just removing the held spinlock does not fit well into the overall lame locking model found in the driver. The thing is that I would prefer to exhibit 'remove custom callbacks' side of the changes as it is done now, and fixing severe 'invalid contex' bugs is left as a valuable side effect. I may attempt to find enough free time to follow your instructions, but frankly speaking I don't see it beneficial to split a single good all-sufficient change into three or more: removal of spinlocks, replacement of phy_start_aneg(), then a non-functional clean-up. Bikeshedding isn't my preference, but a report about technical flaws related to the published changes is appreciated, otherwise let me ask you to accept the changes as is, secondary optimizations can be done on top of them. -- With best wishes, Vladimir
Re: [PATCH 1/6] ravb: remove custom .nway_reset from ethtool ops
Hello Sergei, On 05/24/2018 08:01 PM, Sergei Shtylyov wrote: > On 05/24/2018 07:44 PM, Andrew Lunn wrote: > >> The change fixes a sleep in atomic context issue, which can be >> always triggered by running 'ethtool -r' command, because >> phy_start_aneg() protects phydev fields by a mutex. >>> >>> You don't say that *not* grabbing the spinlock is safe... I say both that it is the fix and it is safe, I've already described the function of the spinlock in my comments, and it is more or less clear from the driver code. >> >> For it to be unsafe, i think that would mean phylib would need to call >> back into the MAC driver? The only way that could happen is via the >> adjust_link call. And that will deadlock, since it takes the same >> lock. >> >> Or am i/we missing something? > >It doesn't take any locks currently, only patches #3/#6 makes it do so... And that's the proper fix in my opinion, my tests don't unveil any issues. -- With best wishes, Vladimir
Re: [PATCH 1/6] ravb: remove custom .nway_reset from ethtool ops
On 05/24/2018 04:22 PM, Andrew Lunn wrote:
> On Thu, May 24, 2018 at 02:11:53PM +0300, Vladimir Zapolskiy wrote:
>> The change fixes a sleep in atomic context issue, which can be
>> always triggered by running 'ethtool -r' command, because
>> phy_start_aneg() protects phydev fields by a mutex.
>>
>> Another note is that the change implicitly replaces phy_start_aneg()
>> with a newer phy_restart_aneg().
>>
>> Signed-off-by: Vladimir Zapolskiy
>> ---
>> drivers/net/ethernet/renesas/ravb_main.c | 17 +
>> 1 file changed, 1 insertion(+), 16 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/renesas/ravb_main.c
>> b/drivers/net/ethernet/renesas/ravb_main.c
>> index 68f122140966..4a043eb0e2aa 100644
>> --- a/drivers/net/ethernet/renesas/ravb_main.c
>> +++ b/drivers/net/ethernet/renesas/ravb_main.c
>> @@ -1150,21 +1150,6 @@ static int ravb_set_link_ksettings(struct net_device
>> *ndev,
>> return error;
>> }
>>
>> -static int ravb_nway_reset(struct net_device *ndev)
>> -{
>> -struct ravb_private *priv = netdev_priv(ndev);
>> -int error = -ENODEV;
>> -unsigned long flags;
>> -
>> -if (ndev->phydev) {
>> -spin_lock_irqsave(&priv->lock, flags);
>> -error = phy_start_aneg(ndev->phydev);
>> -spin_unlock_irqrestore(&priv->lock, flags);
>> -}
>
> Eck! phylib assumes thread context and takes a mutex while calling
> into the PHY driver.
>
> It would be good to add some sort of fixes: tag. Maybe for the commit
> that added the generic nway_reset? That would at least cover some
> stable kernels.
>
> Reviewed-by: Andrew Lunn
>
Hi Andrew, thank you for review.
generally it makes sense to add Fixes tag, but as I said in
the commit message the problem is present before reused phy_ethtool_*()
functions were added to the kernel, so some kind of juggling with
the proper kernel version would be required in assumption that
the fixes are backported as an unmodified changes.
Hopefully Sergei as the driver maintainer can verify the fixes on
older kernels and suggest the right kernel versions for backporting.
--
With best wishes,
Vladimir
Re: [PATCH 3/6] ravb: remove custom .set_link_ksettings from ethtool ops
Hi Andrew, On 05/24/2018 04:29 PM, Andrew Lunn wrote: > On Thu, May 24, 2018 at 02:11:55PM +0300, Vladimir Zapolskiy wrote: >> The change replaces a custom implementation of .set_link_ksettings >> callback with a shared phy_ethtool_set_link_ksettings(), this fixes >> sleep in atomic context bug, which is encountered every time when link >> settings are changed by ethtool. >> >> Now duplex mode setting is enforced in ravb_adjust_link() only, also >> now TX/RX is disabled when link is put down or modifications to E-MAC >> registers ECMR and GECMR are expected for both cases of checked and >> ignored link status pin state from E-MAC interrupt handler. >> >> Signed-off-by: Vladimir Zapolskiy >> --- >> drivers/net/ethernet/renesas/ravb_main.c | 58 >> +--- >> 1 file changed, 15 insertions(+), 43 deletions(-) >> >> diff --git a/drivers/net/ethernet/renesas/ravb_main.c >> b/drivers/net/ethernet/renesas/ravb_main.c >> index 3d91caa44176..0d811c02ff34 100644 >> --- a/drivers/net/ethernet/renesas/ravb_main.c >> +++ b/drivers/net/ethernet/renesas/ravb_main.c >> @@ -980,6 +980,13 @@ static void ravb_adjust_link(struct net_device *ndev) >> struct ravb_private *priv = netdev_priv(ndev); >> struct phy_device *phydev = ndev->phydev; >> bool new_state = false; >> +unsigned long flags; >> + >> +spin_lock_irqsave(&priv->lock, flags); > > Hi Vladimir > > It is pretty unusual to see an adjust_link callback take a lock. Is it > clearly defined what it is protecting? > thank you for review. As the commit message says, the hardware manual claims that any modifications to ECMR and GECMR registers, i.e. calls to ravb_set_duplex() and ravb_set_rate() from the modified ravb_adjust_link() function, has to be done when RX/TX is disabled (same ECMR register bit fields), the spinlock serializes interrupt handlers and modifications to ECMR contents, its previous usage for ethtool handlers was obviously wrong. The information is quite implicit, but the change emphasizes it. -- With best wishes, Vladimir
[PATCH 5/6] sh_eth: remove custom .get_link_ksettings from ethtool ops
The change replaces a custom implementation of .get_link_ksettings
callback with a shared phy_ethtool_get_link_ksettings(), note that
&priv->lock wrapping is not needed, because the lock does not
serialize access to phydev fields.
Signed-off-by: Vladimir Zapolskiy
---
drivers/net/ethernet/renesas/sh_eth.c | 18 +-
1 file changed, 1 insertion(+), 17 deletions(-)
diff --git a/drivers/net/ethernet/renesas/sh_eth.c
b/drivers/net/ethernet/renesas/sh_eth.c
index 6d1fed2b4a4a..e627b2b6c3b3 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -2019,22 +2019,6 @@ static int sh_eth_phy_start(struct net_device *ndev)
return 0;
}
-static int sh_eth_get_link_ksettings(struct net_device *ndev,
-struct ethtool_link_ksettings *cmd)
-{
- struct sh_eth_private *mdp = netdev_priv(ndev);
- unsigned long flags;
-
- if (!ndev->phydev)
- return -ENODEV;
-
- spin_lock_irqsave(&mdp->lock, flags);
- phy_ethtool_ksettings_get(ndev->phydev, cmd);
- spin_unlock_irqrestore(&mdp->lock, flags);
-
- return 0;
-}
-
static int sh_eth_set_link_ksettings(struct net_device *ndev,
const struct ethtool_link_ksettings *cmd)
{
@@ -2411,7 +2395,7 @@ static const struct ethtool_ops sh_eth_ethtool_ops = {
.get_sset_count = sh_eth_get_sset_count,
.get_ringparam = sh_eth_get_ringparam,
.set_ringparam = sh_eth_set_ringparam,
- .get_link_ksettings = sh_eth_get_link_ksettings,
+ .get_link_ksettings = phy_ethtool_get_link_ksettings,
.set_link_ksettings = sh_eth_set_link_ksettings,
.get_wol= sh_eth_get_wol,
.set_wol= sh_eth_set_wol,
--
2.8.1
[PATCH 6/6] sh_eth: remove custom .set_link_ksettings from ethtool ops
The change replaces a custom implementation of .set_link_ksettings
callback with a shared phy_ethtool_set_link_ksettings(), this fixes
sleep in atomic context bug, which is encountered every time when link
settings are changed by ethtool.
Now duplex mode setting is enforced in ravb_adjust_link() only, also
now TX/RX is disabled when link is put down or modifications to E-MAC
registers ECMR and GECMR are expected for both cases of checked and
ignored link status pin state from E-MAC interrupt handler.
Signed-off-by: Vladimir Zapolskiy
---
drivers/net/ethernet/renesas/sh_eth.c | 58 +--
1 file changed, 15 insertions(+), 43 deletions(-)
diff --git a/drivers/net/ethernet/renesas/sh_eth.c
b/drivers/net/ethernet/renesas/sh_eth.c
index e627b2b6c3b3..a3115888bd04 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -1916,8 +1916,15 @@ static void sh_eth_adjust_link(struct net_device *ndev)
{
struct sh_eth_private *mdp = netdev_priv(ndev);
struct phy_device *phydev = ndev->phydev;
+ unsigned long flags;
int new_state = 0;
+ spin_lock_irqsave(&mdp->lock, flags);
+
+ /* Disable TX and RX right over here, if E-MAC change is ignored */
+ if (mdp->cd->no_psr || mdp->no_ether_link)
+ sh_eth_rcv_snd_disable(ndev);
+
if (phydev->link) {
if (phydev->duplex != mdp->duplex) {
new_state = 1;
@@ -1936,18 +1943,21 @@ static void sh_eth_adjust_link(struct net_device *ndev)
sh_eth_modify(ndev, ECMR, ECMR_TXF, 0);
new_state = 1;
mdp->link = phydev->link;
- if (mdp->cd->no_psr || mdp->no_ether_link)
- sh_eth_rcv_snd_enable(ndev);
}
} else if (mdp->link) {
new_state = 1;
mdp->link = 0;
mdp->speed = 0;
mdp->duplex = -1;
- if (mdp->cd->no_psr || mdp->no_ether_link)
- sh_eth_rcv_snd_disable(ndev);
}
+ /* Enable TX and RX right over here, if E-MAC change is ignored */
+ if ((mdp->cd->no_psr || mdp->no_ether_link) && phydev->link)
+ sh_eth_rcv_snd_enable(ndev);
+
+ mmiowb();
+ spin_unlock_irqrestore(&mdp->lock, flags);
+
if (new_state && netif_msg_link(mdp))
phy_print_status(phydev);
}
@@ -2019,44 +2029,6 @@ static int sh_eth_phy_start(struct net_device *ndev)
return 0;
}
-static int sh_eth_set_link_ksettings(struct net_device *ndev,
-const struct ethtool_link_ksettings *cmd)
-{
- struct sh_eth_private *mdp = netdev_priv(ndev);
- unsigned long flags;
- int ret;
-
- if (!ndev->phydev)
- return -ENODEV;
-
- spin_lock_irqsave(&mdp->lock, flags);
-
- /* disable tx and rx */
- sh_eth_rcv_snd_disable(ndev);
-
- ret = phy_ethtool_ksettings_set(ndev->phydev, cmd);
- if (ret)
- goto error_exit;
-
- if (cmd->base.duplex == DUPLEX_FULL)
- mdp->duplex = 1;
- else
- mdp->duplex = 0;
-
- if (mdp->cd->set_duplex)
- mdp->cd->set_duplex(ndev);
-
-error_exit:
- mdelay(1);
-
- /* enable tx and rx */
- sh_eth_rcv_snd_enable(ndev);
-
- spin_unlock_irqrestore(&mdp->lock, flags);
-
- return ret;
-}
-
/* If it is ever necessary to increase SH_ETH_REG_DUMP_MAX_REGS, the
* version must be bumped as well. Just adding registers up to that
* limit is fine, as long as the existing register indices don't
@@ -2396,7 +2368,7 @@ static const struct ethtool_ops sh_eth_ethtool_ops = {
.get_ringparam = sh_eth_get_ringparam,
.set_ringparam = sh_eth_set_ringparam,
.get_link_ksettings = phy_ethtool_get_link_ksettings,
- .set_link_ksettings = sh_eth_set_link_ksettings,
+ .set_link_ksettings = phy_ethtool_set_link_ksettings,
.get_wol= sh_eth_get_wol,
.set_wol= sh_eth_set_wol,
};
--
2.8.1
[PATCH 1/6] ravb: remove custom .nway_reset from ethtool ops
The change fixes a sleep in atomic context issue, which can be
always triggered by running 'ethtool -r' command, because
phy_start_aneg() protects phydev fields by a mutex.
Another note is that the change implicitly replaces phy_start_aneg()
with a newer phy_restart_aneg().
Signed-off-by: Vladimir Zapolskiy
---
drivers/net/ethernet/renesas/ravb_main.c | 17 +
1 file changed, 1 insertion(+), 16 deletions(-)
diff --git a/drivers/net/ethernet/renesas/ravb_main.c
b/drivers/net/ethernet/renesas/ravb_main.c
index 68f122140966..4a043eb0e2aa 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -1150,21 +1150,6 @@ static int ravb_set_link_ksettings(struct net_device
*ndev,
return error;
}
-static int ravb_nway_reset(struct net_device *ndev)
-{
- struct ravb_private *priv = netdev_priv(ndev);
- int error = -ENODEV;
- unsigned long flags;
-
- if (ndev->phydev) {
- spin_lock_irqsave(&priv->lock, flags);
- error = phy_start_aneg(ndev->phydev);
- spin_unlock_irqrestore(&priv->lock, flags);
- }
-
- return error;
-}
-
static u32 ravb_get_msglevel(struct net_device *ndev)
{
struct ravb_private *priv = netdev_priv(ndev);
@@ -1377,7 +1362,7 @@ static int ravb_set_wol(struct net_device *ndev, struct
ethtool_wolinfo *wol)
}
static const struct ethtool_ops ravb_ethtool_ops = {
- .nway_reset = ravb_nway_reset,
+ .nway_reset = phy_ethtool_nway_reset,
.get_msglevel = ravb_get_msglevel,
.set_msglevel = ravb_set_msglevel,
.get_link = ethtool_op_get_link,
--
2.8.1
[PATCH 4/6] sh_eth: remove custom .nway_reset from ethtool ops
The change fixes a sleep in atomic context issue, which can be
always triggered by running 'ethtool -r' command, because
phy_start_aneg() protects phydev fields by a mutex.
Another note is that the change implicitly replaces phy_start_aneg()
with a newer phy_restart_aneg().
Signed-off-by: Vladimir Zapolskiy
---
drivers/net/ethernet/renesas/sh_eth.c | 18 +-
1 file changed, 1 insertion(+), 17 deletions(-)
diff --git a/drivers/net/ethernet/renesas/sh_eth.c
b/drivers/net/ethernet/renesas/sh_eth.c
index d9cadfb1bc4a..6d1fed2b4a4a 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -2252,22 +2252,6 @@ static void sh_eth_get_regs(struct net_device *ndev,
struct ethtool_regs *regs,
pm_runtime_put_sync(&mdp->pdev->dev);
}
-static int sh_eth_nway_reset(struct net_device *ndev)
-{
- struct sh_eth_private *mdp = netdev_priv(ndev);
- unsigned long flags;
- int ret;
-
- if (!ndev->phydev)
- return -ENODEV;
-
- spin_lock_irqsave(&mdp->lock, flags);
- ret = phy_start_aneg(ndev->phydev);
- spin_unlock_irqrestore(&mdp->lock, flags);
-
- return ret;
-}
-
static u32 sh_eth_get_msglevel(struct net_device *ndev)
{
struct sh_eth_private *mdp = netdev_priv(ndev);
@@ -2418,7 +2402,7 @@ static int sh_eth_set_wol(struct net_device *ndev, struct
ethtool_wolinfo *wol)
static const struct ethtool_ops sh_eth_ethtool_ops = {
.get_regs_len = sh_eth_get_regs_len,
.get_regs = sh_eth_get_regs,
- .nway_reset = sh_eth_nway_reset,
+ .nway_reset = phy_ethtool_nway_reset,
.get_msglevel = sh_eth_get_msglevel,
.set_msglevel = sh_eth_set_msglevel,
.get_link = ethtool_op_get_link,
--
2.8.1
[PATCH 0/6] ravb/sh_eth: fix sleep in atomic by reusing shared ethtool handlers
For ages trivial changes to RAVB and SuperH ethernet links by means of standard 'ethtool' trigger a 'sleeping function called from invalid context' bug, to visualize it on r8a7795 ULCB: % ethtool -r eth0 BUG: sleeping function called from invalid context at kernel/locking/mutex.c:747 in_atomic(): 1, irqs_disabled(): 128, pid: 554, name: ethtool INFO: lockdep is turned off. irq event stamp: 0 hardirqs last enabled at (0): [<>] (null) hardirqs last disabled at (0): [] copy_process.isra.7.part.8+0x2cc/0x1918 softirqs last enabled at (0): [] copy_process.isra.7.part.8+0x2cc/0x1918 softirqs last disabled at (0): [<>] (null) CPU: 5 PID: 554 Comm: ethtool Not tainted 4.17.0-rc4-arm64-renesas+ #33 Hardware name: Renesas H3ULCB board based on r8a7795 ES2.0+ (DT) Call trace: dump_backtrace+0x0/0x198 show_stack+0x24/0x30 dump_stack+0xb8/0xf4 ___might_sleep+0x1c8/0x1f8 __might_sleep+0x58/0x90 __mutex_lock+0x50/0x890 mutex_lock_nested+0x3c/0x50 phy_start_aneg_priv+0x38/0x180 phy_start_aneg+0x24/0x30 ravb_nway_reset+0x3c/0x68 dev_ethtool+0x3dc/0x2338 dev_ioctl+0x19c/0x490 sock_do_ioctl+0xe0/0x238 sock_ioctl+0x254/0x460 do_vfs_ioctl+0xb0/0x918 ksys_ioctl+0x50/0x80 sys_ioctl+0x34/0x48 __sys_trace_return+0x0/0x4 The root cause is that an attempt to modify ECMR and GECMR registers only when RX/TX function is disabled was too overcomplicated in its original implementation, also processing of an optional Link Change interrupt added even more complexity, as a result the implementation was error prone. The new locking scheme is confirmed to be correct by dumping driver specific and generic PHY framework function calls with aid of ftrace while running more or less advanced tests. Please note that sh_eth patches from the series were built-tested only. On purpose I do not add Fixes tags, the reused PHY handlers were added way later than the fixed problems were firstly found in the drivers. Vladimir Zapolskiy (6): ravb: remove custom .nway_reset from ethtool ops ravb: remove custom .get_link_ksettings from ethtool ops ravb: remove custom .set_link_ksettings from ethtool ops sh_eth: remove custom .nway_reset from ethtool ops sh_eth: remove custom .get_link_ksettings from ethtool ops sh_eth: remove custom .set_link_ksettings from ethtool ops drivers/net/ethernet/renesas/ravb_main.c | 93 ++- drivers/net/ethernet/renesas/sh_eth.c| 94 ++-- 2 files changed, 34 insertions(+), 153 deletions(-) -- 2.8.1
[PATCH 2/6] ravb: remove custom .get_link_ksettings from ethtool ops
The change replaces a custom implementation of .get_link_ksettings
callback with a shared phy_ethtool_get_link_ksettings(), note that
&priv->lock wrapping is not needed, because the lock does not
serialize access to phydev fields.
Signed-off-by: Vladimir Zapolskiy
---
drivers/net/ethernet/renesas/ravb_main.c | 18 +-
1 file changed, 1 insertion(+), 17 deletions(-)
diff --git a/drivers/net/ethernet/renesas/ravb_main.c
b/drivers/net/ethernet/renesas/ravb_main.c
index 4a043eb0e2aa..3d91caa44176 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -1096,22 +1096,6 @@ static int ravb_phy_start(struct net_device *ndev)
return 0;
}
-static int ravb_get_link_ksettings(struct net_device *ndev,
- struct ethtool_link_ksettings *cmd)
-{
- struct ravb_private *priv = netdev_priv(ndev);
- unsigned long flags;
-
- if (!ndev->phydev)
- return -ENODEV;
-
- spin_lock_irqsave(&priv->lock, flags);
- phy_ethtool_ksettings_get(ndev->phydev, cmd);
- spin_unlock_irqrestore(&priv->lock, flags);
-
- return 0;
-}
-
static int ravb_set_link_ksettings(struct net_device *ndev,
const struct ethtool_link_ksettings *cmd)
{
@@ -1372,7 +1356,7 @@ static const struct ethtool_ops ravb_ethtool_ops = {
.get_ringparam = ravb_get_ringparam,
.set_ringparam = ravb_set_ringparam,
.get_ts_info= ravb_get_ts_info,
- .get_link_ksettings = ravb_get_link_ksettings,
+ .get_link_ksettings = phy_ethtool_get_link_ksettings,
.set_link_ksettings = ravb_set_link_ksettings,
.get_wol= ravb_get_wol,
.set_wol= ravb_set_wol,
--
2.8.1
[PATCH 3/6] ravb: remove custom .set_link_ksettings from ethtool ops
The change replaces a custom implementation of .set_link_ksettings
callback with a shared phy_ethtool_set_link_ksettings(), this fixes
sleep in atomic context bug, which is encountered every time when link
settings are changed by ethtool.
Now duplex mode setting is enforced in ravb_adjust_link() only, also
now TX/RX is disabled when link is put down or modifications to E-MAC
registers ECMR and GECMR are expected for both cases of checked and
ignored link status pin state from E-MAC interrupt handler.
Signed-off-by: Vladimir Zapolskiy
---
drivers/net/ethernet/renesas/ravb_main.c | 58 +---
1 file changed, 15 insertions(+), 43 deletions(-)
diff --git a/drivers/net/ethernet/renesas/ravb_main.c
b/drivers/net/ethernet/renesas/ravb_main.c
index 3d91caa44176..0d811c02ff34 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -980,6 +980,13 @@ static void ravb_adjust_link(struct net_device *ndev)
struct ravb_private *priv = netdev_priv(ndev);
struct phy_device *phydev = ndev->phydev;
bool new_state = false;
+ unsigned long flags;
+
+ spin_lock_irqsave(&priv->lock, flags);
+
+ /* Disable TX and RX right over here, if E-MAC change is ignored */
+ if (priv->no_avb_link)
+ ravb_rcv_snd_disable(ndev);
if (phydev->link) {
if (phydev->duplex != priv->duplex) {
@@ -997,18 +1004,21 @@ static void ravb_adjust_link(struct net_device *ndev)
ravb_modify(ndev, ECMR, ECMR_TXF, 0);
new_state = true;
priv->link = phydev->link;
- if (priv->no_avb_link)
- ravb_rcv_snd_enable(ndev);
}
} else if (priv->link) {
new_state = true;
priv->link = 0;
priv->speed = 0;
priv->duplex = -1;
- if (priv->no_avb_link)
- ravb_rcv_snd_disable(ndev);
}
+ /* Enable TX and RX right over here, if E-MAC change is ignored */
+ if (priv->no_avb_link && phydev->link)
+ ravb_rcv_snd_enable(ndev);
+
+ mmiowb();
+ spin_unlock_irqrestore(&priv->lock, flags);
+
if (new_state && netif_msg_link(priv))
phy_print_status(phydev);
}
@@ -1096,44 +1106,6 @@ static int ravb_phy_start(struct net_device *ndev)
return 0;
}
-static int ravb_set_link_ksettings(struct net_device *ndev,
- const struct ethtool_link_ksettings *cmd)
-{
- struct ravb_private *priv = netdev_priv(ndev);
- unsigned long flags;
- int error;
-
- if (!ndev->phydev)
- return -ENODEV;
-
- spin_lock_irqsave(&priv->lock, flags);
-
- /* Disable TX and RX */
- ravb_rcv_snd_disable(ndev);
-
- error = phy_ethtool_ksettings_set(ndev->phydev, cmd);
- if (error)
- goto error_exit;
-
- if (cmd->base.duplex == DUPLEX_FULL)
- priv->duplex = 1;
- else
- priv->duplex = 0;
-
- ravb_set_duplex(ndev);
-
-error_exit:
- mdelay(1);
-
- /* Enable TX and RX */
- ravb_rcv_snd_enable(ndev);
-
- mmiowb();
- spin_unlock_irqrestore(&priv->lock, flags);
-
- return error;
-}
-
static u32 ravb_get_msglevel(struct net_device *ndev)
{
struct ravb_private *priv = netdev_priv(ndev);
@@ -1357,7 +1329,7 @@ static const struct ethtool_ops ravb_ethtool_ops = {
.set_ringparam = ravb_set_ringparam,
.get_ts_info= ravb_get_ts_info,
.get_link_ksettings = phy_ethtool_get_link_ksettings,
- .set_link_ksettings = ravb_set_link_ksettings,
+ .set_link_ksettings = phy_ethtool_set_link_ksettings,
.get_wol= ravb_get_wol,
.set_wol= ravb_set_wol,
};
--
2.8.1
Re: [PATCH] ARM: dts: add phy-reset property for rk3066a-rayeager emac
Hello Chris,
On 11/07/2017 04:49 AM, Chris Zhong wrote:
> The ethernet phy of rk3066a-rayeager has a reset pin, it controlled by
> GPIO1_D6, this pin should be pull down then pull up to reset the phy.
> Add a phy-reset property in emac, make the phy can be reset when emac
> power on.
for PHY reset there are properties 'reset-gpios' and 'reset-delay-us',
please reference to Documentation/devicetree/bindings/net/mdio.txt
Can you try to reuse them instead of adding new custom properties?
As a side question, which is mainly addressed to Sergei and Roger,
I don't quite understand why PHY properties were initially added to
MAC/MDIO bus device tree nodes, in my opinion they must be moved under
PHY device tree nodes.
--
With best wishes,
Vladimir
>
> Signed-off-by: Chris Zhong
> ---
>
> arch/arm/boot/dts/rk3066a-rayeager.dts | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/arch/arm/boot/dts/rk3066a-rayeager.dts
> b/arch/arm/boot/dts/rk3066a-rayeager.dts
> index 570157f..6064a0a 100644
> --- a/arch/arm/boot/dts/rk3066a-rayeager.dts
> +++ b/arch/arm/boot/dts/rk3066a-rayeager.dts
> @@ -173,6 +173,8 @@
> pinctrl-0 = <&emac_xfer>, <&emac_mdio>, <&rmii_rst>;
> phy = <&phy0>;
> phy-supply = <&vcc_rmii>;
> + phy-reset-gpios = <&gpio1 RK_PD6 GPIO_ACTIVE_LOW>; /* PHY_RST */
> + phy-reset-duration = <10>; /* millisecond */
> status = "okay";
>
> phy0: ethernet-phy@0 {
>
Re: [PATCH v2] net: lpc_eth: Check clk_prepare_enable() error
Hi Fabio,
On 23.08.2016 13:38, Fabio Estevam wrote:
> From: Fabio Estevam
>
> clk_prepare_enable() may fail, so we should better check its return
> value and propagate it in the case of failure
>
> While at it, replace __lpc_eth_clock_enable() with a plain
> clk_prepare_enable/clk_disable_unprepare() call in order to
> simplify the code.
>
> Signed-off-by: Fabio Estevam
> ---
> Changes since v1:
> - Replace __lpc_eth_clock_enable() with a plain
> clk_prepare_enable/clk_disable_unprepare() call (Vladimir)
>
> drivers/net/ethernet/nxp/lpc_eth.c | 19 ---
> 1 file changed, 8 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/ethernet/nxp/lpc_eth.c
> b/drivers/net/ethernet/nxp/lpc_eth.c
> index 4d4ecba..cda87ba 100644
> --- a/drivers/net/ethernet/nxp/lpc_eth.c
> +++ b/drivers/net/ethernet/nxp/lpc_eth.c
> @@ -475,14 +475,6 @@ static void __lpc_get_mac(struct netdata_local *pldat,
> u8 *mac)
> mac[5] = tmp >> 8;
> }
>
> -static void __lpc_eth_clock_enable(struct netdata_local *pldat, bool enable)
> -{
> - if (enable)
> - clk_prepare_enable(pldat->clk);
> - else
> - clk_disable_unprepare(pldat->clk);
> -}
> -
> static void __lpc_params_setup(struct netdata_local *pldat)
> {
> u32 tmp;
> @@ -1056,7 +1048,7 @@ static int lpc_eth_close(struct net_device *ndev)
> writel(0, LPC_ENET_MAC2(pldat->net_base));
> spin_unlock_irqrestore(&pldat->lock, flags);
>
> - __lpc_eth_clock_enable(pldat, false);
> + clk_disable_unprepare(pldat->clk);
>
> return 0;
> }
> @@ -1197,11 +1189,14 @@ static int lpc_eth_ioctl(struct net_device *ndev,
> struct ifreq *req, int cmd)
> static int lpc_eth_open(struct net_device *ndev)
> {
> struct netdata_local *pldat = netdev_priv(ndev);
> + int ret;
>
> if (netif_msg_ifup(pldat))
> dev_dbg(&pldat->pdev->dev, "enabling %s\n", ndev->name);
>
> - __lpc_eth_clock_enable(pldat, true);
> + ret = clk_prepare_enable(pldat->clk);
> + if (ret)
> + return ret;
>
> /* Suspended PHY makes LPC ethernet core block, so resume now */
> phy_resume(ndev->phydev);
> @@ -1320,7 +1315,9 @@ static int lpc_eth_drv_probe(struct platform_device
> *pdev)
> }
>
> /* Enable network clock */
> - __lpc_eth_clock_enable(pldat, true);
> + ret = clk_prepare_enable(pldat->clk);
> + if (ret)
> + goto err_out_free_dev;
>
> /* Map IO space */
> pldat->net_base = ioremap(res->start, resource_size(res));
>
sorry for nitpicking, would you mind to send v3 with a clk_put(pldat->clk)
resource release call on newly added error path?
In advance if it is done, please feel free to add my
Acked-by: Vladimir Zapolskiy
Thank you for the fix!
--
With best wishes,
Vladimir
Re: [PATCH] net: lpc_eth: Check clk_prepare_enable() error
Hi Fabio,
On 23.08.2016 05:01, Fabio Estevam wrote:
> From: Fabio Estevam
>
> clk_prepare_enable() may fail, so we should better check its return
> value and propagate it in the case of failure.
>
> Signed-off-by: Fabio Estevam
> ---
> drivers/net/ethernet/nxp/lpc_eth.c | 29 ++---
> 1 file changed, 22 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/ethernet/nxp/lpc_eth.c
> b/drivers/net/ethernet/nxp/lpc_eth.c
> index 4d4ecba..7c796f7 100644
> --- a/drivers/net/ethernet/nxp/lpc_eth.c
> +++ b/drivers/net/ethernet/nxp/lpc_eth.c
> @@ -475,12 +475,19 @@ static void __lpc_get_mac(struct netdata_local *pldat,
> u8 *mac)
> mac[5] = tmp >> 8;
> }
>
> -static void __lpc_eth_clock_enable(struct netdata_local *pldat, bool enable)
> +static int __lpc_eth_clock_enable(struct netdata_local *pldat, bool enable)
> {
> - if (enable)
> - clk_prepare_enable(pldat->clk);
> - else
> + int ret;
> +
> + if (enable) {
> + ret = clk_prepare_enable(pldat->clk);
> + if (ret)
> + return ret;
> + } else {
> clk_disable_unprepare(pldat->clk);
> + }
> +
> + return 0;
> }
The change is correct, but let me ask you to replace all instances of
__lpc_eth_clock_enable() with plain clk_prepare_enable() or
clk_disable_unprepare(). It should save one redundant error check also.
--
With best wishes,
Vladimir
[PATCH] net: mediatek: fix checking for NULL instead of IS_ERR() in .probe
devm_ioremap_resource() returns ERR_PTR() value on error, it never
returns NULL, fix it and propagate the returned error upwards.
Fixes: 656e705243fd ("net-next: mediatek: add support for MT7623 ethernet")
Signed-off-by: Vladimir Zapolskiy
---
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 7f2126b..e0b68af 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -1690,8 +1690,8 @@ static int mtk_probe(struct platform_device *pdev)
return -ENOMEM;
eth->base = devm_ioremap_resource(&pdev->dev, res);
- if (!eth->base)
- return -EADDRNOTAVAIL;
+ if (IS_ERR(eth->base))
+ return PTR_ERR(eth->base);
spin_lock_init(ð->page_lock);
--
2.1.4
Re: [PATCH 2/2] of_mdio: use PTR_ERR_OR_ZERO()
Hello Sergei,
On 12.03.2016 00:13, Sergei Shtylyov wrote:
> PTR_ERR_OR_ZERO() is open coded in of_phy_register_fixed_link(), so just
> call it directly...
>
> Signed-off-by: Sergei Shtylyov
>
> ---
> drivers/of/of_mdio.c |6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> Index: net-next/drivers/of/of_mdio.c
> ===
> --- net-next.orig/drivers/of/of_mdio.c
> +++ net-next/drivers/of/of_mdio.c
> @@ -412,7 +412,7 @@ int of_phy_register_fixed_link(struct de
> if (strcmp(managed, "in-band-status") == 0) {
> /* status is zeroed, namely its .link member */
> phy = fixed_phy_register(PHY_POLL, &status, -1, np);
> - return IS_ERR(phy) ? PTR_ERR(phy) : 0;
> + return PTR_ERR_OR_ZERO(phy);
> }
> }
>
> @@ -434,7 +434,7 @@ int of_phy_register_fixed_link(struct de
> return -EPROBE_DEFER;
>
> phy = fixed_phy_register(PHY_POLL, &status, link_gpio, np);
> - return IS_ERR(phy) ? PTR_ERR(phy) : 0;
> + return PTR_ERR_OR_ZERO(phy);
> }
>
> /* Old binding */
> @@ -446,7 +446,7 @@ int of_phy_register_fixed_link(struct de
> status.pause = be32_to_cpu(fixed_link_prop[3]);
> status.asym_pause = be32_to_cpu(fixed_link_prop[4]);
> phy = fixed_phy_register(PHY_POLL, &status, -1, np);
> - return IS_ERR(phy) ? PTR_ERR(phy) : 0;
> + return PTR_ERR_OR_ZERO(phy);
> }
>
> return -ENODEV;
>
Reviewed-by: Vladimir Zapolskiy
--
With best wishes,
Vladimir
Re: [PATCH 1/2] of_mdio: use IS_ERR_OR_NULL()
Hello Sergei,
On 12.03.2016 00:12, Sergei Shtylyov wrote:
> IS_ERR_OR_NULL() is open coded in of_mdiobus_register_{phy|device}(), so
> just call it directly...
>
> Signed-off-by: Sergei Shtylyov
>
> ---
> drivers/of/of_mdio.c |4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> Index: net-next/drivers/of/of_mdio.c
> ===
> --- net-next.orig/drivers/of/of_mdio.c
> +++ net-next/drivers/of/of_mdio.c
> @@ -56,7 +56,7 @@ static int of_mdiobus_register_phy(struc
> phy = phy_device_create(mdio, addr, phy_id, 0, NULL);
> else
> phy = get_phy_device(mdio, addr, is_c45);
> - if (!phy || IS_ERR(phy))
> + if (IS_ERR_OR_NULL(phy))
> return 1;
>
> rc = irq_of_parse_and_map(child, 0);
> @@ -98,7 +98,7 @@ static int of_mdiobus_register_device(st
> int rc;
>
> mdiodev = mdio_device_create(mdio, addr);
> - if (!mdiodev || IS_ERR(mdiodev))
> + if (IS_ERR_OR_NULL(mdiodev))
> return 1;
>
I don't see how it is possible for mdio_device_create() to return NULL,
instead of this change consider to remove if (!mdiodev) check.
> /* Associate the OF node with the device structure so it
>
--
With best wishes,
Vladimir
[PATCH] net: lpc_eth: remove irq > NR_IRQS check from probe()
If the driver is used on an ARM platform with SPARSE_IRQ defined,
semantics of NR_IRQS is different (minimal value of virtual irqs) and
by default it is set to 16, see arch/arm/include/asm/irq.h.
This value may be less than the actual number of virtual irqs, which
may break the driver initialization. The check removal allows to use
the driver on such a platform, and, if irq controller driver works
correctly, the check is not needed on legacy platforms.
Fixes a runtime problem:
lpc-eth 3106.ethernet: error getting resources.
lpc_eth: lpc-eth: not found (-6).
Signed-off-by: Vladimir Zapolskiy
---
drivers/net/ethernet/nxp/lpc_eth.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/nxp/lpc_eth.c
b/drivers/net/ethernet/nxp/lpc_eth.c
index b159ef8..0576651 100644
--- a/drivers/net/ethernet/nxp/lpc_eth.c
+++ b/drivers/net/ethernet/nxp/lpc_eth.c
@@ -1326,7 +1326,7 @@ static int lpc_eth_drv_probe(struct platform_device *pdev)
/* Get platform resources */
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
irq = platform_get_irq(pdev, 0);
- if ((!res) || (irq < 0) || (irq >= NR_IRQS)) {
+ if (!res || irq < 0) {
dev_err(&pdev->dev, "error getting resources.\n");
ret = -ENXIO;
goto err_exit;
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] net: lpc_eth: fix warnings caused by enabling unprepared clock
If common clock framework is configured, the driver generates warnings,
which are fixed by this change:
WARNING: CPU: 0 PID: 1 at linux/drivers/clk/clk.c:727
clk_core_enable+0x2c/0xa4()
Modules linked in:
CPU: 0 PID: 1 Comm: swapper Not tainted 4.3.0-rc2+ #141
Hardware name: LPC32XX SoC (Flattened Device Tree)
Backtrace:
[<>] (dump_backtrace) from [<>] (show_stack+0x18/0x1c)
[<>] (show_stack) from [<>] (dump_stack+0x20/0x28)
[<>] (dump_stack) from [<>] (warn_slowpath_common+0x90/0xb8)
[<>] (warn_slowpath_common) from [<>] (warn_slowpath_null+0x24/0x2c)
[<>] (warn_slowpath_null) from [<>] (clk_core_enable+0x2c/0xa4)
[<>] (clk_core_enable) from [<>] (clk_enable+0x24/0x38)
[<>] (clk_enable) from [<>] (lpc_eth_drv_probe+0xfc/0x99c)
[<>] (lpc_eth_drv_probe) from [<>] (platform_drv_probe+0x50/0xa0)
[<>] (platform_drv_probe) from [<>] (driver_probe_device+0x18c/0x408)
[<>] (driver_probe_device) from [<>] (__driver_attach+0x70/0x94)
[<>] (__driver_attach) from [<>] (bus_for_each_dev+0x74/0x98)
[<>] (bus_for_each_dev) from [<>] (driver_attach+0x20/0x28)
[<>] (driver_attach) from [<>] (bus_add_driver+0x11c/0x248)
[<>] (bus_add_driver) from [<>] (driver_register+0xa4/0xe8)
[<>] (driver_register) from [<>] (__platform_driver_register+0x50/0x64)
[<>] (__platform_driver_register) from [<>] (lpc_eth_driver_init+0x18/0x20)
[<>] (lpc_eth_driver_init) from [<>] (do_one_initcall+0x11c/0x1dc)
[<>] (do_one_initcall) from [<>] (kernel_init_freeable+0x10c/0x1d4)
[<>] (kernel_init_freeable) from [<>] (kernel_init+0x10/0xec)
[<>] (kernel_init) from [<>] (ret_from_fork+0x14/0x24)
Signed-off-by: Vladimir Zapolskiy
---
drivers/net/ethernet/nxp/lpc_eth.c | 13 ++---
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/nxp/lpc_eth.c
b/drivers/net/ethernet/nxp/lpc_eth.c
index 66fd868..b159ef8 100644
--- a/drivers/net/ethernet/nxp/lpc_eth.c
+++ b/drivers/net/ethernet/nxp/lpc_eth.c
@@ -476,13 +476,12 @@ static void __lpc_get_mac(struct netdata_local *pldat, u8
*mac)
mac[5] = tmp >> 8;
}
-static void __lpc_eth_clock_enable(struct netdata_local *pldat,
- bool enable)
+static void __lpc_eth_clock_enable(struct netdata_local *pldat, bool enable)
{
if (enable)
- clk_enable(pldat->clk);
+ clk_prepare_enable(pldat->clk);
else
- clk_disable(pldat->clk);
+ clk_disable_unprepare(pldat->clk);
}
static void __lpc_params_setup(struct netdata_local *pldat)
@@ -1494,7 +1493,7 @@ err_out_free_irq:
err_out_iounmap:
iounmap(pldat->net_base);
err_out_disable_clocks:
- clk_disable(pldat->clk);
+ clk_disable_unprepare(pldat->clk);
clk_put(pldat->clk);
err_out_free_dev:
free_netdev(ndev);
@@ -1519,7 +1518,7 @@ static int lpc_eth_drv_remove(struct platform_device
*pdev)
iounmap(pldat->net_base);
mdiobus_unregister(pldat->mii_bus);
mdiobus_free(pldat->mii_bus);
- clk_disable(pldat->clk);
+ clk_disable_unprepare(pldat->clk);
clk_put(pldat->clk);
free_netdev(ndev);
@@ -1540,7 +1539,7 @@ static int lpc_eth_drv_suspend(struct platform_device
*pdev,
if (netif_running(ndev)) {
netif_device_detach(ndev);
__lpc_eth_shutdown(pldat);
- clk_disable(pldat->clk);
+ clk_disable_unprepare(pldat->clk);
/*
* Reset again now clock is disable to be sure
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3.14] net: sysctl_net_core: remove compiler warning about unused variable 'one'
From: Dirk Behme
Remove the compiler warning
net/core/sysctl_net_core.c:26:12: warning: 'one' defined but not used
[-Wunused-variable]
static int one = 1;
introduced by the 3.14.37 stable commit a1d55b36de6bf2 ("net: sysctl_net_core:
check SNDBUF and RCVBUF for min length") which made 'one' obsolete (in
contrast to recent mainline kernel, which still uses 'one').
Signed-off-by: Dirk Behme
Signed-off-by: Vladimir Zapolskiy
Cc:
Cc: # 3.14.x-
Fixes: a1d55b36de6b ("net: sysctl_net_core: check SNDBUF and RCVBUF for min
length")
---
net/core/sysctl_net_core.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c
index e731c96..8725b91 100644
--- a/net/core/sysctl_net_core.c
+++ b/net/core/sysctl_net_core.c
@@ -23,7 +23,6 @@
#include
static int zero = 0;
-static int one = 1;
static int ushort_max = USHRT_MAX;
static int min_sndbuf = SOCK_MIN_SNDBUF;
static int min_rcvbuf = SOCK_MIN_RCVBUF;
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] net: phy: spi_ks8995: clean up ks8995_registers_read/write
The change removes redundant sysfs binary file boundary checks,
since this task is already done on caller side in fs/sysfs/file.c
Signed-off-by: Vladimir Zapolskiy
---
drivers/net/phy/spi_ks8995.c | 22 --
1 file changed, 22 deletions(-)
diff --git a/drivers/net/phy/spi_ks8995.c b/drivers/net/phy/spi_ks8995.c
index 4653015..f091d69 100644
--- a/drivers/net/phy/spi_ks8995.c
+++ b/drivers/net/phy/spi_ks8995.c
@@ -209,8 +209,6 @@ static int ks8995_reset(struct ks8995_switch *ks)
return ks8995_start(ks);
}
-/* */
-
static ssize_t ks8995_registers_read(struct file *filp, struct kobject *kobj,
struct bin_attribute *bin_attr, char *buf, loff_t off, size_t count)
{
@@ -220,19 +218,9 @@ static ssize_t ks8995_registers_read(struct file *filp,
struct kobject *kobj,
dev = container_of(kobj, struct device, kobj);
ks8995 = dev_get_drvdata(dev);
- if (unlikely(off > ks8995->regs_attr.size))
- return 0;
-
- if ((off + count) > ks8995->regs_attr.size)
- count = ks8995->regs_attr.size - off;
-
- if (unlikely(!count))
- return count;
-
return ks8995_read(ks8995, buf, off, count);
}
-
static ssize_t ks8995_registers_write(struct file *filp, struct kobject *kobj,
struct bin_attribute *bin_attr, char *buf, loff_t off, size_t count)
{
@@ -242,19 +230,9 @@ static ssize_t ks8995_registers_write(struct file *filp,
struct kobject *kobj,
dev = container_of(kobj, struct device, kobj);
ks8995 = dev_get_drvdata(dev);
- if (unlikely(off >= ks8995->regs_attr.size))
- return -EFBIG;
-
- if ((off + count) > ks8995->regs_attr.size)
- count = ks8995->regs_attr.size - off;
-
- if (unlikely(!count))
- return count;
-
return ks8995_write(ks8995, buf, off, count);
}
-
static const struct bin_attribute ks8995_registers_attr = {
.attr = {
.name = "registers",
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] net: qlcnic: clean up sysfs error codes
Hello David, On 29.05.2015 02:28, David Miller wrote: > From: Vladimir Zapolskiy > Date: Tue, 26 May 2015 03:49:45 +0300 > >> Replace confusing QL_STATUS_INVALID_PARAM == -1 == -EPERM with -EINVAL >> and QLC_STATUS_UNSUPPORTED_CMD == -2 == -ENOENT with -EOPNOTSUPP, the >> latter error code is arguable, but it is already used in the driver, >> so let it be here as well. >> >> Also remove always false (!buf) check on read(), the driver should >> not care if userspace gets its EFAULT or not. >> >> Signed-off-by: Vladimir Zapolskiy > > Qlogic folks, I'm waiting for your promised feedback. > Rajesh reviewed and acked the change, thank you. http://www.spinics.net/lists/netdev/msg331073.html -- With best wishes, Vladimir -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] net: netxen: correct sysfs bin attribute return code
If read() syscall requests unexpected number of bytes from "dimm" binary
attribute file, return EINVAL instead of EPERM.
At the same time pin down sysfs file size to the fixed
sizeof(struct netxen_dimm_cfg), which allows to exploit some missing
sanity checks from kernfs (file boundary checks vs offset etc.)
Signed-off-by: Vladimir Zapolskiy
---
drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
index e0c31e3..6409a06 100644
--- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
+++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
@@ -3025,9 +3025,9 @@ netxen_sysfs_read_dimm(struct file *filp, struct kobject
*kobj,
u8 dw, rows, cols, banks, ranks;
u32 val;
- if (size != sizeof(struct netxen_dimm_cfg)) {
+ if (size < attr->size) {
netdev_err(netdev, "Invalid size\n");
- return -1;
+ return -EINVAL;
}
memset(&dimm, 0, sizeof(struct netxen_dimm_cfg));
@@ -3137,7 +3137,7 @@ out:
static struct bin_attribute bin_attr_dimm = {
.attr = { .name = "dimm", .mode = (S_IRUGO | S_IWUSR) },
- .size = 0,
+ .size = sizeof(struct netxen_dimm_cfg),
.read = netxen_sysfs_read_dimm,
};
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] net: qlcnic: clean up sysfs error codes
Replace confusing QL_STATUS_INVALID_PARAM == -1 == -EPERM with -EINVAL
and QLC_STATUS_UNSUPPORTED_CMD == -2 == -ENOENT with -EOPNOTSUPP, the
latter error code is arguable, but it is already used in the driver,
so let it be here as well.
Also remove always false (!buf) check on read(), the driver should
not care if userspace gets its EFAULT or not.
Signed-off-by: Vladimir Zapolskiy
---
drivers/net/ethernet/qlogic/qlcnic/qlcnic.h | 3 -
drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c | 2 +-
drivers/net/ethernet/qlogic/qlcnic/qlcnic_sysfs.c | 77 +++
3 files changed, 36 insertions(+), 46 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h
b/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h
index f221126..055f376 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h
@@ -1326,9 +1326,6 @@ struct qlcnic_eswitch {
};
-/* Return codes for Error handling */
-#define QL_STATUS_INVALID_PARAM-1
-
#define MAX_BW 100 /* % of link speed */
#define MIN_BW 1 /* % of link speed */
#define MAX_VLAN_ID4095
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
index 367f397..2f6cc42 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
@@ -1031,7 +1031,7 @@ int qlcnic_init_pci_info(struct qlcnic_adapter *adapter)
pfn = pci_info[i].id;
if (pfn >= ahw->max_vnic_func) {
- ret = QL_STATUS_INVALID_PARAM;
+ ret = -EINVAL;
dev_err(&adapter->pdev->dev, "%s: Invalid function
0x%x, max 0x%x\n",
__func__, pfn, ahw->max_vnic_func);
goto err_eswitch;
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sysfs.c
b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sysfs.c
index 59a721f..05c28f2 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sysfs.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sysfs.c
@@ -24,8 +24,6 @@
#include
#endif
-#define QLC_STATUS_UNSUPPORTED_CMD -2
-
int qlcnicvf_config_bridged_mode(struct qlcnic_adapter *adapter, u32 enable)
{
return -EOPNOTSUPP;
@@ -166,7 +164,7 @@ static int qlcnic_82xx_store_beacon(struct qlcnic_adapter
*adapter,
u8 b_state, b_rate;
if (len != sizeof(u16))
- return QL_STATUS_INVALID_PARAM;
+ return -EINVAL;
memcpy(&beacon, buf, sizeof(u16));
err = qlcnic_validate_beacon(adapter, beacon, &b_state, &b_rate);
@@ -383,17 +381,17 @@ static int validate_pm_config(struct qlcnic_adapter
*adapter,
dest_pci_func = pm_cfg[i].dest_npar;
src_index = qlcnic_is_valid_nic_func(adapter, src_pci_func);
if (src_index < 0)
- return QL_STATUS_INVALID_PARAM;
+ return -EINVAL;
dest_index = qlcnic_is_valid_nic_func(adapter, dest_pci_func);
if (dest_index < 0)
- return QL_STATUS_INVALID_PARAM;
+ return -EINVAL;
s_esw_id = adapter->npars[src_index].phy_port;
d_esw_id = adapter->npars[dest_index].phy_port;
if (s_esw_id != d_esw_id)
- return QL_STATUS_INVALID_PARAM;
+ return -EINVAL;
}
return 0;
@@ -414,7 +412,7 @@ static ssize_t qlcnic_sysfs_write_pm_config(struct file
*filp,
count = size / sizeof(struct qlcnic_pm_func_cfg);
rem = size % sizeof(struct qlcnic_pm_func_cfg);
if (rem)
- return QL_STATUS_INVALID_PARAM;
+ return -EINVAL;
qlcnic_swap32_buffer((u32 *)buf, size / sizeof(u32));
pm_cfg = (struct qlcnic_pm_func_cfg *)buf;
@@ -427,7 +425,7 @@ static ssize_t qlcnic_sysfs_write_pm_config(struct file
*filp,
action = !!pm_cfg[i].action;
index = qlcnic_is_valid_nic_func(adapter, pci_func);
if (index < 0)
- return QL_STATUS_INVALID_PARAM;
+ return -EINVAL;
id = adapter->npars[index].phy_port;
ret = qlcnic_config_port_mirroring(adapter, id,
@@ -440,7 +438,7 @@ static ssize_t qlcnic_sysfs_write_pm_config(struct file
*filp,
pci_func = pm_cfg[i].pci_func;
index = qlcnic_is_valid_nic_func(adapter, pci_func);
if (index < 0)
- return QL_STATUS_INVALID_PARAM;
+ return -EINVAL;
id = adapter->npars[index].phy_port;
adapter->npars[index].enable_pm = !!pm_cfg[i].action;
adapter->npars[index].dest_npar = id;
@@
