Re: [PATCH net-next v2] net: axienet: Remove redundant dev_err call in axienet_probe()
On 3/29/21 3:45 AM, Huang Guobin wrote:
> From: Guobin Huang
>
> There is a error message within devm_ioremap_resource
> already, so remove the dev_err call to avoid redundant
> error message.
>
> Reported-by: Hulk Robot
> Signed-off-by: Guobin Huang
> ---
> drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> index 5d677db0aee5..f77a794540fc 100644
> --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> @@ -1878,7 +1878,6 @@ static int axienet_probe(struct platform_device *pdev)
> ethres = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> lp->regs = devm_ioremap_resource(&pdev->dev, ethres);
> if (IS_ERR(lp->regs)) {
> - dev_err(&pdev->dev, "could not map Axi Ethernet regs.\n");
> ret = PTR_ERR(lp->regs);
> goto cleanup_clk;
> }
>
I have already reviewed v1 and I can't see any v2 description what has
changed. Why am I getting this v2? Where is my tag?
Thanks,
Michal
[PATCH] can: xilinx_can: Simplify code by using dev_err_probe()
Use already prepared dev_err_probe() introduced by commit a787e5400a1c
("driver core: add device probe log helper").
It simplifies EPROBE_DEFER handling.
Also unify message format for similar error cases.
Signed-off-by: Michal Simek
---
drivers/net/can/xilinx_can.c | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/net/can/xilinx_can.c b/drivers/net/can/xilinx_can.c
index 37fa19c62d73..3b883e607d8b 100644
--- a/drivers/net/can/xilinx_can.c
+++ b/drivers/net/can/xilinx_can.c
@@ -1772,17 +1772,15 @@ static int xcan_probe(struct platform_device *pdev)
/* Getting the CAN can_clk info */
priv->can_clk = devm_clk_get(&pdev->dev, "can_clk");
if (IS_ERR(priv->can_clk)) {
- if (PTR_ERR(priv->can_clk) != -EPROBE_DEFER)
- dev_err(&pdev->dev, "Device clock not found.\n");
- ret = PTR_ERR(priv->can_clk);
+ ret = dev_err_probe(&pdev->dev, PTR_ERR(priv->can_clk),
+ "device clock not found\n");
goto err_free;
}
priv->bus_clk = devm_clk_get(&pdev->dev, devtype->bus_clk_name);
if (IS_ERR(priv->bus_clk)) {
- if (PTR_ERR(priv->bus_clk) != -EPROBE_DEFER)
- dev_err(&pdev->dev, "bus clock not found\n");
- ret = PTR_ERR(priv->bus_clk);
+ ret = dev_err_probe(&pdev->dev, PTR_ERR(priv->bus_clk),
+ "bus clock not found\n");
goto err_free;
}
--
2.30.0
Re: [PATCH net-next 3/3] drivers: net: xilinx_emaclite: Add COMPILE_TEST support
On 31. 10. 20 18:47, Andrew Lunn wrote: > To improve build testing of this driver, add COMPILE_TEST support. > > Signed-off-by: Andrew Lunn > --- > drivers/net/ethernet/xilinx/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/xilinx/Kconfig > b/drivers/net/ethernet/xilinx/Kconfig > index d0d0d4fe9d40..3b2137d1f4c6 100644 > --- a/drivers/net/ethernet/xilinx/Kconfig > +++ b/drivers/net/ethernet/xilinx/Kconfig > @@ -18,7 +18,7 @@ if NET_VENDOR_XILINX > > config XILINX_EMACLITE > tristate "Xilinx 10/100 Ethernet Lite support" > - depends on PPC32 || MICROBLAZE || ARCH_ZYNQ || MIPS > + depends on PPC32 || MICROBLAZE || ARCH_ZYNQ || MIPS || COMPILE_TEST > select PHYLIB > help > This driver supports the 10/100 Ethernet Lite from Xilinx. > Acked-by: Michal Simek Thanks, Michal
Re: [PATCH net-next 2/3] drivers: net: xilinx_emaclite: Fix -Wpointer-to-int-cast warnings with W=1
On 31. 10. 20 18:47, Andrew Lunn wrote: > drivers/net/ethernet//xilinx/xilinx_emaclite.c:341:35: warning: cast from > pointer to integer of different size [-Wpointer-to-int-cast] > 341 | addr = (void __iomem __force *)((u32 __force)addr ^ > > Use long instead of u32 to avoid problems on 64 bit systems. > > Signed-off-by: Andrew Lunn > --- > drivers/net/ethernet/xilinx/xilinx_emaclite.c | 10 +- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/net/ethernet/xilinx/xilinx_emaclite.c > b/drivers/net/ethernet/xilinx/xilinx_emaclite.c > index 2c98e4cc07a5..f56c1fd01061 100644 > --- a/drivers/net/ethernet/xilinx/xilinx_emaclite.c > +++ b/drivers/net/ethernet/xilinx/xilinx_emaclite.c > @@ -97,7 +97,7 @@ > #define ALIGNMENT4 > > /* BUFFER_ALIGN(adr) calculates the number of bytes to the next alignment. */ > -#define BUFFER_ALIGN(adr) ((ALIGNMENT - ((u32)adr)) % ALIGNMENT) > +#define BUFFER_ALIGN(adr) ((ALIGNMENT - ((long)adr)) % ALIGNMENT) I can't see any reason to change unsigned type to signed one. > > #ifdef __BIG_ENDIAN > #define xemaclite_readl ioread32be > @@ -338,7 +338,7 @@ static int xemaclite_send_data(struct net_local *drvdata, > u8 *data, >* if it is configured in HW >*/ > > - addr = (void __iomem __force *)((u32 __force)addr ^ > + addr = (void __iomem __force *)((long __force)addr ^ ditto. >XEL_BUFFER_OFFSET); > reg_data = xemaclite_readl(addr + XEL_TSR_OFFSET); > > @@ -399,7 +399,7 @@ static u16 xemaclite_recv_data(struct net_local *drvdata, > u8 *data, int maxlen) >* will correct on subsequent calls >*/ > if (drvdata->rx_ping_pong != 0) > - addr = (void __iomem __force *)((u32 __force)addr ^ > + addr = (void __iomem __force *)((long __force)addr ^ ditto. >XEL_BUFFER_OFFSET); > else > return 0; /* No data was available */ > @@ -1192,9 +1192,9 @@ static int xemaclite_of_probe(struct platform_device > *ofdev) > } > > dev_info(dev, > - "Xilinx EmacLite at 0x%08X mapped to 0x%08X, irq=%d\n", > + "Xilinx EmacLite at 0x%08X mapped to 0x%08lX, irq=%d\n", >(unsigned int __force)ndev->mem_start, > - (unsigned int __force)lp->base_addr, ndev->irq); > + (unsigned long __force)lp->base_addr, ndev->irq); This is different case but I don't think address can be signed type here too. > return 0; > > error: > Thanks, Michal
Re: [PATCH net-next 1/3] drivers: net: xilinx_emaclite: Add missing parameter kerneldoc
On 31. 10. 20 18:47, Andrew Lunn wrote:
> The txqueue parameter to the watchdog callback is unused in this
> driver. But it still needs to be documented.
>
> Signed-off-by: Andrew Lunn
> ---
> drivers/net/ethernet/xilinx/xilinx_emaclite.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/net/ethernet/xilinx/xilinx_emaclite.c
> b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
> index 0c26f5bcc523..2c98e4cc07a5 100644
> --- a/drivers/net/ethernet/xilinx/xilinx_emaclite.c
> +++ b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
> @@ -518,6 +518,7 @@ static int xemaclite_set_mac_address(struct net_device
> *dev, void *address)
> /**
> * xemaclite_tx_timeout - Callback for Tx Timeout
> * @dev: Pointer to the network device
> + * @txqueue: Unused
> *
> * This function is called when Tx time out occurs for Emaclite device.
> */
>
Fixes: 0290bd291cc0 ("netdev: pass the stuck queue to the timeout handler")
Reviewed-by: Michal Simek
Thanks,
Michal
[PATCH 1/3] can: xilinx_can: Limit CANFD brp to 2
From: Srinivas Neeli
Bit enlarging is observed for CANFD2.0 when brp is 1,
So change brp_min value to 2.
Signed-off-by: Srinivas Neeli
Signed-off-by: Michal Simek
---
drivers/net/can/xilinx_can.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/can/xilinx_can.c b/drivers/net/can/xilinx_can.c
index c1dbab8c896d..f4b544b69646 100644
--- a/drivers/net/can/xilinx_can.c
+++ b/drivers/net/can/xilinx_can.c
@@ -259,7 +259,7 @@ static const struct can_bittiming_const
xcan_bittiming_const_canfd2 = {
.tseg2_min = 1,
.tseg2_max = 128,
.sjw_max = 128,
- .brp_min = 1,
+ .brp_min = 2,
.brp_max = 256,
.brp_inc = 1,
};
@@ -272,7 +272,7 @@ static struct can_bittiming_const
xcan_data_bittiming_const_canfd2 = {
.tseg2_min = 1,
.tseg2_max = 16,
.sjw_max = 16,
- .brp_min = 1,
+ .brp_min = 2,
.brp_max = 256,
.brp_inc = 1,
};
--
2.28.0
[PATCH 3/3] can: xilinx_can: Fix incorrect variable and initialize with a default value
From: Srinivas Neeli Some variables with incorrect type were passed to "of_property_read_u32" API, "of_property_read_u32" API was expecting an "u32 *" but the formal parameter that was passed was of type "int *". Fixed the issue by changing the variable types from "int" to "u32" and initialized with a default value. Fixed sparse warning. Addresses-Coverity: "incompatible_param" Addresses-Coverity: "UNINIT(Using uninitialized value)" Signed-off-by: Srinivas Neeli Signed-off-by: Michal Simek --- drivers/net/can/xilinx_can.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/can/xilinx_can.c b/drivers/net/can/xilinx_can.c index 3393e2a73e15..46c04b6390f8 100644 --- a/drivers/net/can/xilinx_can.c +++ b/drivers/net/can/xilinx_can.c @@ -1671,7 +1671,7 @@ static int xcan_probe(struct platform_device *pdev) void __iomem *addr; int ret; int rx_max, tx_max; - int hw_tx_max, hw_rx_max; + u32 hw_tx_max = 0, hw_rx_max = 0; const char *hw_tx_max_property; /* Get the virtual base address for the device */ @@ -1724,7 +1724,7 @@ static int xcan_probe(struct platform_device *pdev) */ if (!(devtype->flags & XCAN_FLAG_TX_MAILBOXES) && (devtype->flags & XCAN_FLAG_TXFEMP)) - tx_max = min(hw_tx_max, 2); + tx_max = min(hw_tx_max, 2U); else tx_max = 1; -- 2.28.0
[PATCH 0/3] can: xilinx_can: Some minor changes
Hi, recently some small patches come to our internal tree. We started to use coverity which found 2 issues (last two patches) which is simply to fix. Thanks, Michal Srinivas Neeli (3): can: xilinx_can: Limit CANFD brp to 2 can: xilinx_can: Check return value of set_reset_mode can: xilinx_can: Fix incorrect variable and initialize with a default value drivers/net/can/xilinx_can.c | 14 +- 1 file changed, 9 insertions(+), 5 deletions(-) -- 2.28.0
[PATCH 2/3] can: xilinx_can: Check return value of set_reset_mode
From: Srinivas Neeli
Check return value of set_reset_mode() for error.
Addresses-Coverity: "check_return"
Signed-off-by: Srinivas Neeli
Signed-off-by: Michal Simek
---
drivers/net/can/xilinx_can.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/net/can/xilinx_can.c b/drivers/net/can/xilinx_can.c
index f4b544b69646..3393e2a73e15 100644
--- a/drivers/net/can/xilinx_can.c
+++ b/drivers/net/can/xilinx_can.c
@@ -1369,9 +1369,13 @@ static irqreturn_t xcan_interrupt(int irq, void *dev_id)
static void xcan_chip_stop(struct net_device *ndev)
{
struct xcan_priv *priv = netdev_priv(ndev);
+ int ret;
/* Disable interrupts and leave the can in configuration mode */
- set_reset_mode(ndev);
+ ret = set_reset_mode(ndev);
+ if (ret < 0)
+ netdev_dbg(ndev, "set_reset_mode() Failed\n");
+
priv->can.state = CAN_STATE_STOPPED;
}
--
2.28.0
Re: xilinx_axienet_main.c:undefined reference to `devm_ioremap_resource'
On 23. 06. 20 23:27, Brendan Higgins wrote: > On Sat, Jun 20, 2020 at 1:59 AM kernel test robot wrote: >> >> Hi Brendan, >> >> It's probably a bug fix that unveils the link errors. >> >> tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git >> master >> head: 4333a9b0b67bb4e8bcd91bdd80da80b0ec151162 >> commit: 1af73a25e6e7d9f2f1e2a14259cc9ffce6d8f6d4 staging: exfat: fix >> multiple definition error of `rename_file' >> date: 6 months ago >> config: um-allyesconfig (attached as .config) >> compiler: gcc-9 (Debian 9.3.0-13) 9.3.0 >> reproduce (this is a W=1 build): >> git checkout 1af73a25e6e7d9f2f1e2a14259cc9ffce6d8f6d4 >> # save the attached .config to linux build tree >> make W=1 ARCH=um >> >> If you fix the issue, kindly add following tag as appropriate >> Reported-by: kernel test robot >> >> All errors (new ones prefixed by >>): >> >>/usr/bin/ld: drivers/net/ethernet/xilinx/xilinx_axienet_main.o: in >> function `axienet_probe': xilinx_axienet_main.c:(.text+0x1aa6): undefined reference to `devm_ioremap_resource' /usr/bin/ld: xilinx_axienet_main.c:(.text+0x1d06): undefined reference to `devm_ioremap_resource' >>/usr/bin/ld: xilinx_axienet_main.c:(.text+0x2001): undefined reference to >> `devm_ioremap_resource' >>collect2: error: ld returned 1 exit status > > I posted a fix for this months ago: > > https://patchwork.ozlabs.org/project/linux-um/patch/20191211192742.95699-4-brendanhigg...@google.com/ > > I thought it got picked up by the maintainer. > Can you please send it again? Thanks, Michal
Re: [PATCH] net: axienet: fix spelling mistake in comment "Exteneded" -> "extended"
On 15. 06. 20 10:29, Colin King wrote: > From: Colin Ian King > > There is a spelling mistake in a comment. Fix it. > > Signed-off-by: Colin Ian King > --- > drivers/net/ethernet/xilinx/xilinx_axienet.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet.h > b/drivers/net/ethernet/xilinx/xilinx_axienet.h > index fbaf3c987d9c..f34c7903ff52 100644 > --- a/drivers/net/ethernet/xilinx/xilinx_axienet.h > +++ b/drivers/net/ethernet/xilinx/xilinx_axienet.h > @@ -186,7 +186,7 @@ > #define XAE_RAF_TXVSTRPMODE_MASK 0x0180 /* Tx VLAN STRIP mode */ > #define XAE_RAF_RXVSTRPMODE_MASK 0x0600 /* Rx VLAN STRIP mode */ > #define XAE_RAF_NEWFNCENBL_MASK 0x0800 /* New function mode > */ > -/* Exteneded Multicast Filtering mode */ > +/* Extended Multicast Filtering mode */ > #define XAE_RAF_EMULTIFLTRENBL_MASK 0x1000 > #define XAE_RAF_STATSRST_MASK0x2000 /* Stats. Counter > Reset */ > #define XAE_RAF_RXBADFRMEN_MASK 0x4000 /* Recv Bad Frame > Enable */ > Acked-by: Michal Simek Thanks, Michal
Re: [PATCH RESEND net] net: phy: xgmiitorgmii: Support generic PHY status read
On 21. 02. 19 12:03, Michal Simek wrote: > On 21. 02. 19 11:24, Paul Kocialkowski wrote: >> Hi, >> >> On Wed, 2019-02-20 at 07:58 +0100, Michal Simek wrote: >>> Hi, >>> >>> On 19. 02. 19 18:25, Andrew Lunn wrote: >>>>> Thanks for the suggestion! So I had a closer look at that driver to try >>>>> and see what could go wrong and it looks like I found a few things >>>>> there. >>>> >>>> Hi Paul >>>> >>>> Yes, this driver has issues. If i remember correctly, it got merged >>>> while i was on vacation. I pointed out a few issues, but the authors >>>> never responded. Feel free to fix it up. >>> >>> Will be good to know who was that person. >>> >>> I can't do much this week with this because responsible person for this >>> driver is out of office this week. That's why please give us some time >>> to get back to this. >> >> Understood. I think we need to start a discussion about how the general >> design of this driver can be improved. >> >> In particular, I wonder if it could work better to make this driver a >> PHY driver that just redirects all its ops to the actual PHY driver, >> except for read_status where it should also add some code. > > I didn't take a look at Linux driver but it should work in a way that it > checks description (more below) and then wait for attached phy to do its > work and on the way back just setup this bridge based on that. > >> Maybe we could also manage to expose a RGMII PHY mode to the actual PHY >> this way. Currently, the PHY mode has to be set to GMII for the MAC to >> be configured correctly, but the PHY also gets this information while >> it should be told that RGMII is in use. This doesn't seem to play a big >> role in PHY configuration though, but it's still inadequate. >> >> What do you think? > > I stop the driver to be applied to u-boot because exactly this > gmii/rgmii configuration. There is a need that mac is configured to gmii > and this needs to be checked but to phy rgmii needs to be exposed. > I was trying to find out hardware design and board where I can do some > experiments but didn't find out. That's why I need to wait for > colleagues to point me to that. Harini: Can you please respond this thread? Thanks, Michal
Re: [PATCH RESEND net] net: phy: xgmiitorgmii: Support generic PHY status read
On 21. 02. 19 11:24, Paul Kocialkowski wrote: > Hi, > > On Wed, 2019-02-20 at 07:58 +0100, Michal Simek wrote: >> Hi, >> >> On 19. 02. 19 18:25, Andrew Lunn wrote: >>>> Thanks for the suggestion! So I had a closer look at that driver to try >>>> and see what could go wrong and it looks like I found a few things >>>> there. >>> >>> Hi Paul >>> >>> Yes, this driver has issues. If i remember correctly, it got merged >>> while i was on vacation. I pointed out a few issues, but the authors >>> never responded. Feel free to fix it up. >> >> Will be good to know who was that person. >> >> I can't do much this week with this because responsible person for this >> driver is out of office this week. That's why please give us some time >> to get back to this. > > Understood. I think we need to start a discussion about how the general > design of this driver can be improved. > > In particular, I wonder if it could work better to make this driver a > PHY driver that just redirects all its ops to the actual PHY driver, > except for read_status where it should also add some code. I didn't take a look at Linux driver but it should work in a way that it checks description (more below) and then wait for attached phy to do its work and on the way back just setup this bridge based on that. > Maybe we could also manage to expose a RGMII PHY mode to the actual PHY > this way. Currently, the PHY mode has to be set to GMII for the MAC to > be configured correctly, but the PHY also gets this information while > it should be told that RGMII is in use. This doesn't seem to play a big > role in PHY configuration though, but it's still inadequate. > > What do you think? I stop the driver to be applied to u-boot because exactly this gmii/rgmii configuration. There is a need that mac is configured to gmii and this needs to be checked but to phy rgmii needs to be exposed. I was trying to find out hardware design and board where I can do some experiments but didn't find out. That's why I need to wait for colleagues to point me to that. Thanks, Michal
Re: [PATCH RESEND net] net: phy: xgmiitorgmii: Support generic PHY status read
Hi, On 19. 02. 19 18:25, Andrew Lunn wrote: >> Thanks for the suggestion! So I had a closer look at that driver to try >> and see what could go wrong and it looks like I found a few things >> there. > > Hi Paul > > Yes, this driver has issues. If i remember correctly, it got merged > while i was on vacation. I pointed out a few issues, but the authors > never responded. Feel free to fix it up. Will be good to know who was that person. I can't do much this week with this because responsible person for this driver is out of office this week. That's why please give us some time to get back to this. Thanks, Michal
Re: [RFC PATCH 2/2] net: macb: Disable TX checksum offloading on all Zynq
On 4.6.2018 17:06, Nicolas Ferre wrote: > On 25/05/2018 at 23:44, Jennifer Dahm wrote: >> The Zynq ethernet hardware has checksum offloading bugs that cause >> small UDP packets (<= 2 bytes) to be sent with an incorrect checksum >> (0x) and forwarded UDP packets to be re-checksummed, which is >> illegal behavior. The best solution we have right now is to disable >> hardware TX checksum offloading entirely. >> >> Signed-off-by: Jennifer Dahm > > Adding some xilinx people I know... Harini: Please look at this. Thanks, Michal
Re: [PATCH 3/3] can: xilinx: fix xcan_start_xmit()'s return type
On 27.4.2018 09:55, Marc Kleine-Budde wrote:
> On 04/27/2018 09:49 AM, Michal Simek wrote:
>> On 26.4.2018 23:13, Luc Van Oostenryck wrote:
>>> The method ndo_start_xmit() is defined as returning an 'netdev_tx_t',
>>> which is a typedef for an enum type, but the implementation in this
>>> driver returns an 'int'.
>>>
>>> Fix this by returning 'netdev_tx_t' in this driver too.
>>>
>>> Signed-off-by: Luc Van Oostenryck
>>> ---
>>> drivers/net/can/xilinx_can.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/can/xilinx_can.c b/drivers/net/can/xilinx_can.c
>>> index 89aec07c2..a19648606 100644
>>> --- a/drivers/net/can/xilinx_can.c
>>> +++ b/drivers/net/can/xilinx_can.c
>>> @@ -386,7 +386,7 @@ static int xcan_do_set_mode(struct net_device *ndev,
>>> enum can_mode mode)
>>> *
>>> * Return: 0 on success and failure value on error
>>> */
>>> -static int xcan_start_xmit(struct sk_buff *skb, struct net_device *ndev)
>>> +static netdev_tx_t xcan_start_xmit(struct sk_buff *skb, struct net_device
>>> *ndev)
>>> {
>>> struct xcan_priv *priv = netdev_priv(ndev);
>>> struct net_device_stats *stats = &ndev->stats;
>>>
>>
>> It was applied already but there should be also kernel-doc update too to
>> use enum values instead of 0.
>
> Like this:
>
>> diff --git a/drivers/net/can/xilinx_can.c b/drivers/net/can/xilinx_can.c
>> index f07ce4945356..d0ad1473f689 100644
>> --- a/drivers/net/can/xilinx_can.c
>> +++ b/drivers/net/can/xilinx_can.c
>> @@ -398,7 +398,7 @@ static int xcan_do_set_mode(struct net_device *ndev,
>> enum can_mode mode)
>> * function uses the next available free txbuff and populates their fields
>> to
>> * start the transmission.
>> *
>> - * Return: 0 on success and failure value on error
>> + * Return: NETDEV_TX_OK on success and NETDEV_TX_BUSY when the tx queue is
>> full
>> */
>> static netdev_tx_t xcan_start_xmit(struct sk_buff *skb, struct net_device
>> *ndev)
>> {
>
> I can squash in that change.
looks good to me.
Acked-by: Michal Simek
Thanks,
Michal
Re: [PATCH 3/3] can: xilinx: fix xcan_start_xmit()'s return type
On 26.4.2018 23:13, Luc Van Oostenryck wrote:
> The method ndo_start_xmit() is defined as returning an 'netdev_tx_t',
> which is a typedef for an enum type, but the implementation in this
> driver returns an 'int'.
>
> Fix this by returning 'netdev_tx_t' in this driver too.
>
> Signed-off-by: Luc Van Oostenryck
> ---
> drivers/net/can/xilinx_can.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/can/xilinx_can.c b/drivers/net/can/xilinx_can.c
> index 89aec07c2..a19648606 100644
> --- a/drivers/net/can/xilinx_can.c
> +++ b/drivers/net/can/xilinx_can.c
> @@ -386,7 +386,7 @@ static int xcan_do_set_mode(struct net_device *ndev, enum
> can_mode mode)
> *
> * Return: 0 on success and failure value on error
> */
> -static int xcan_start_xmit(struct sk_buff *skb, struct net_device *ndev)
> +static netdev_tx_t xcan_start_xmit(struct sk_buff *skb, struct net_device
> *ndev)
> {
> struct xcan_priv *priv = netdev_priv(ndev);
> struct net_device_stats *stats = &ndev->stats;
>
It was applied already but there should be also kernel-doc update too to
use enum values instead of 0.
M
Re: [PATCH] can: xilinx: fix xcan_start_xmit()'s return type
On 24.4.2018 15:16, Luc Van Oostenryck wrote:
> The method ndo_start_xmit() is defined as returning an 'netdev_tx_t',
> which is a typedef for an enum type, but the implementation in this
> driver returns an 'int'.
>
> Fix this by returning 'netdev_tx_t' in this driver too.
>
> Signed-off-by: Luc Van Oostenryck
> ---
> drivers/net/can/xilinx_can.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/can/xilinx_can.c b/drivers/net/can/xilinx_can.c
> index 89aec07c2..a19648606 100644
> --- a/drivers/net/can/xilinx_can.c
> +++ b/drivers/net/can/xilinx_can.c
> @@ -386,7 +386,7 @@ static int xcan_do_set_mode(struct net_device *ndev, enum
> can_mode mode)
> *
> * Return: 0 on success and failure value on error
> */
> -static int xcan_start_xmit(struct sk_buff *skb, struct net_device *ndev)
> +static netdev_tx_t xcan_start_xmit(struct sk_buff *skb, struct net_device
> *ndev)
> {
> struct xcan_priv *priv = netdev_priv(ndev);
> struct net_device_stats *stats = &ndev->stats;
>
Can you please also align kernel-doc format above?
I see that the whole function is already returning proper enum values.
Thanks,
Michal
Re: [PATCH] net: ethernet: xilinx: Mark XILINX_LL_TEMAC broken on 64-bit
On 29.11.2017 11:01, Geert Uytterhoeven wrote: > On 64-bit (e.g. powerpc64/allmodconfig): > > drivers/net/ethernet/xilinx/ll_temac_main.c: In function > 'temac_start_xmit_done': > drivers/net/ethernet/xilinx/ll_temac_main.c:633:22: warning: cast to > pointer from integer of different size [-Wint-to-pointer-cast] > dev_kfree_skb_irq((struct sk_buff *)cur_p->app4); > ^ > > cdmac_bd.app4 is u32, so it is too small to hold a kernel pointer. > > Note that several other fields in struct cdmac_bd are also too small to > hold physical addresses on 64-bit platforms. > > Signed-off-by: Geert Uytterhoeven > --- > drivers/net/ethernet/xilinx/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/net/ethernet/xilinx/Kconfig > b/drivers/net/ethernet/xilinx/Kconfig > index 6d68c8a8f4f2ac7f..da4ec575ccf9ba4a 100644 > --- a/drivers/net/ethernet/xilinx/Kconfig > +++ b/drivers/net/ethernet/xilinx/Kconfig > @@ -34,6 +34,7 @@ config XILINX_AXI_EMAC > config XILINX_LL_TEMAC > tristate "Xilinx LL TEMAC (LocalLink Tri-mode Ethernet MAC) driver" > depends on (PPC || MICROBLAZE) > + depends on !64BIT || BROKEN > select PHYLIB > ---help--- > This driver supports the Xilinx 10/100/1000 LocalLink TEMAC > Yes. This IP is available only on big endian 32bit systems. Thanks, Michal
Re: [PATCH] net: phy: Fix mask value write on gmii2rgmii converter speed register.
On 14.9.2017 16:34, Andrew Lunn wrote: > On Thu, Sep 14, 2017 at 12:46:31PM +0530, Fahad Kunnathadi wrote: >> To clear Speed Selection in MDIO control register(0x10), >> ie, clear bits 6 and 13 to zero while keeping other bits same. >> Before AND operation,The Mask value has to be perform with bitwise NOT >> operation (ie, ~ operator) >> >> This patch clears current speed selection before writing the >> new speed settings to gmii2rgmii converter > > Hi Fahad > > I expect you will find other issues with this driver. I pointed some > out at the time it is submitted, but the developers went quiet as soon > as it was accepted. Can you please point me to that email? I will create ticket about it in our system to get them resolved. Thanks, Michal
Re: [PATCH v2] net: phy: Use tab for indentation in Kconfig
On 11.8.2017 15:33, Andrew Lunn wrote: > On Fri, Aug 11, 2017 at 02:48:53PM +0200, Michal Simek wrote: >> Using tabs instead of space for indentaion >> >> Signed-off-by: Michal Simek >> Reviewed-by: Andrew Lunn > > Hi Michal > > Thanks for rebasing. > > FYI: > > It is normal to put net-next in the patch subject to indicate this: > > [Patch v2 net-next] Using tabs instead of space for indentaion > > Ah! I just noticed. 'indentation' has a second T. ok. I have sent v3 with this fix and add net-next there. Thanks, Michal
[PATCH net-next v3] net: phy: Use tab for indentation in Kconfig
Using tabs instead of space for indentation. Signed-off-by: Michal Simek Reviewed-by: Andrew Lunn --- Changes in v3: - Fix commit message s/indentaion/indentation./ reported-by Andrew Lunn - Rebase on the top of net-next. HEAD commit: f5b589488ea5ed3bb6168b1a4e7f7b95841d8513 Changes in v2: - Rebased on the top of net-next. HEAD commit: 3b2b69efeca734b78bc85fd02253b0465bb2bec7 Suggested by David Miller drivers/net/phy/Kconfig | 42 +- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig index 8c919203291a..5afe6fdcc968 100644 --- a/drivers/net/phy/Kconfig +++ b/drivers/net/phy/Kconfig @@ -5,7 +5,7 @@ menuconfig MDIO_DEVICE tristate "MDIO bus device drivers" help - MDIO devices and driver infrastructure code. + MDIO devices and driver infrastructure code. config MDIO_BUS tristate @@ -117,11 +117,11 @@ config MDIO_I2C This is library mode. config MDIO_MOXART -tristate "MOXA ART MDIO interface support" -depends on ARCH_MOXART -help - This driver supports the MDIO interface found in the network - interface units of the MOXA ART SoC + tristate "MOXA ART MDIO interface support" + depends on ARCH_MOXART + help + This driver supports the MDIO interface found in the network + interface units of the MOXA ART SoC config MDIO_OCTEON tristate "Octeon and some ThunderX SOCs MDIO buses" @@ -192,7 +192,7 @@ config LED_TRIGGER_PHY state change will trigger the events, for consumption by an LED class driver. There are triggers for each link speed currently supported by the phy, and are of the form: - :: + :: Where speed is in the form: Mbps or Gbps @@ -211,9 +211,9 @@ config AMD_PHY Currently supports the am79c874 config AQUANTIA_PHY -tristate "Aquantia PHYs" ----help--- - Currently supports the Aquantia AQ1202, AQ2104, AQR105, AQR405 + tristate "Aquantia PHYs" + ---help--- + Currently supports the Aquantia AQ1202, AQ2104, AQR105, AQR405 config AT803X_PHY tristate "AT803X PHYs" @@ -382,21 +382,21 @@ config STE10XP This is the driver for the STe100p and STe101p PHYs. config TERANETICS_PHY -tristate "Teranetics PHYs" ----help--- - Currently supports the Teranetics TN2020 + tristate "Teranetics PHYs" + ---help--- + Currently supports the Teranetics TN2020 config VITESSE_PHY -tristate "Vitesse PHYs" ----help--- - Currently supports the vsc8244 + tristate "Vitesse PHYs" + ---help--- + Currently supports the vsc8244 config XILINX_GMII2RGMII - tristate "Xilinx GMII2RGMII converter driver" - ---help--- - This driver support xilinx GMII to RGMII IP core it provides - the Reduced Gigabit Media Independent Interface(RGMII) between - Ethernet physical media devices and the Gigabit Ethernet controller. + tristate "Xilinx GMII2RGMII converter driver" + ---help--- + This driver support xilinx GMII to RGMII IP core it provides + the Reduced Gigabit Media Independent Interface(RGMII) between + Ethernet physical media devices and the Gigabit Ethernet controller. endif # PHYLIB -- 1.9.1
Re: [PATCH] net: phy: Use tab for indentation in Kconfig
On 9.8.2017 06:05, David Miller wrote: > From: Michal Simek > Date: Tue, 8 Aug 2017 11:32:25 +0200 > >> Using tabs instead of space for indentaion >> >> Signed-off-by: Michal Simek > > This really isn't appropriate for the 'net' tree, it doesn't fix > anything it just makes the spacing consistent. > > Please respin this patch against net-next, thank you. v2 sent rebased on the top of net-next. Thanks, Michal -- Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91 w: www.monstr.eu p: +42-0-721842854 Maintainer of Linux kernel - Xilinx Microblaze Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP SoCs signature.asc Description: OpenPGP digital signature
[PATCH v2] net: phy: Use tab for indentation in Kconfig
Using tabs instead of space for indentaion Signed-off-by: Michal Simek Reviewed-by: Andrew Lunn --- Changes in v2: - Rebased on the top of net-next. HEAD commit: 3b2b69efeca734b78bc85fd02253b0465bb2bec7 Suggested by David Miller drivers/net/phy/Kconfig | 42 +- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig index bf73969a9d2b..81683a9677e9 100644 --- a/drivers/net/phy/Kconfig +++ b/drivers/net/phy/Kconfig @@ -5,7 +5,7 @@ menuconfig MDIO_DEVICE tristate "MDIO bus device drivers" help - MDIO devices and driver infrastructure code. + MDIO devices and driver infrastructure code. config MDIO_BUS tristate @@ -117,11 +117,11 @@ config MDIO_I2C This is library mode. config MDIO_MOXART -tristate "MOXA ART MDIO interface support" -depends on ARCH_MOXART -help - This driver supports the MDIO interface found in the network - interface units of the MOXA ART SoC + tristate "MOXA ART MDIO interface support" + depends on ARCH_MOXART + help + This driver supports the MDIO interface found in the network + interface units of the MOXA ART SoC config MDIO_OCTEON tristate "Octeon and some ThunderX SOCs MDIO buses" @@ -192,7 +192,7 @@ config LED_TRIGGER_PHY state change will trigger the events, for consumption by an LED class driver. There are triggers for each link speed currently supported by the phy, and are of the form: - :: + :: Where speed is in the form: Mbps or Gbps @@ -211,9 +211,9 @@ config AMD_PHY Currently supports the am79c874 config AQUANTIA_PHY -tristate "Aquantia PHYs" ----help--- - Currently supports the Aquantia AQ1202, AQ2104, AQR105, AQR405 + tristate "Aquantia PHYs" + ---help--- + Currently supports the Aquantia AQ1202, AQ2104, AQR105, AQR405 config AT803X_PHY tristate "AT803X PHYs" @@ -377,21 +377,21 @@ config STE10XP This is the driver for the STe100p and STe101p PHYs. config TERANETICS_PHY -tristate "Teranetics PHYs" ----help--- - Currently supports the Teranetics TN2020 + tristate "Teranetics PHYs" + ---help--- + Currently supports the Teranetics TN2020 config VITESSE_PHY -tristate "Vitesse PHYs" ----help--- - Currently supports the vsc8244 + tristate "Vitesse PHYs" + ---help--- + Currently supports the vsc8244 config XILINX_GMII2RGMII - tristate "Xilinx GMII2RGMII converter driver" - ---help--- - This driver support xilinx GMII to RGMII IP core it provides - the Reduced Gigabit Media Independent Interface(RGMII) between - Ethernet physical media devices and the Gigabit Ethernet controller. + tristate "Xilinx GMII2RGMII converter driver" + ---help--- + This driver support xilinx GMII to RGMII IP core it provides + the Reduced Gigabit Media Independent Interface(RGMII) between + Ethernet physical media devices and the Gigabit Ethernet controller. endif # PHYLIB -- 1.9.1
[PATCH] net: phy: Use tab for indentation in Kconfig
Using tabs instead of space for indentaion Signed-off-by: Michal Simek --- drivers/net/phy/Kconfig | 42 +- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig index 928fd892f167..5d8cce3fab57 100644 --- a/drivers/net/phy/Kconfig +++ b/drivers/net/phy/Kconfig @@ -5,7 +5,7 @@ menuconfig MDIO_DEVICE tristate "MDIO bus device drivers" help - MDIO devices and driver infrastructure code. + MDIO devices and driver infrastructure code. config MDIO_BUS tristate @@ -107,11 +107,11 @@ config MDIO_HISI_FEMAC Hisilicon SoC that have an Fast Ethernet MAC. config MDIO_MOXART -tristate "MOXA ART MDIO interface support" -depends on ARCH_MOXART -help - This driver supports the MDIO interface found in the network - interface units of the MOXA ART SoC + tristate "MOXA ART MDIO interface support" + depends on ARCH_MOXART + help + This driver supports the MDIO interface found in the network + interface units of the MOXA ART SoC config MDIO_OCTEON tristate "Octeon and some ThunderX SOCs MDIO buses" @@ -172,7 +172,7 @@ config LED_TRIGGER_PHY state change will trigger the events, for consumption by an LED class driver. There are triggers for each link speed currently supported by the phy, and are of the form: - :: + :: Where speed is in the form: Mbps or Gbps @@ -186,9 +186,9 @@ config AMD_PHY Currently supports the am79c874 config AQUANTIA_PHY -tristate "Aquantia PHYs" ----help--- - Currently supports the Aquantia AQ1202, AQ2104, AQR105, AQR405 + tristate "Aquantia PHYs" + ---help--- + Currently supports the Aquantia AQ1202, AQ2104, AQR105, AQR405 config AT803X_PHY tristate "AT803X PHYs" @@ -352,21 +352,21 @@ config STE10XP This is the driver for the STe100p and STe101p PHYs. config TERANETICS_PHY -tristate "Teranetics PHYs" ----help--- - Currently supports the Teranetics TN2020 + tristate "Teranetics PHYs" + ---help--- + Currently supports the Teranetics TN2020 config VITESSE_PHY -tristate "Vitesse PHYs" ----help--- - Currently supports the vsc8244 + tristate "Vitesse PHYs" + ---help--- + Currently supports the vsc8244 config XILINX_GMII2RGMII - tristate "Xilinx GMII2RGMII converter driver" - ---help--- - This driver support xilinx GMII to RGMII IP core it provides - the Reduced Gigabit Media Independent Interface(RGMII) between - Ethernet physical media devices and the Gigabit Ethernet controller. + tristate "Xilinx GMII2RGMII converter driver" + ---help--- + This driver support xilinx GMII to RGMII IP core it provides + the Reduced Gigabit Media Independent Interface(RGMII) between + Ethernet physical media devices and the Gigabit Ethernet controller. endif # PHYLIB -- 1.9.1
[PATCH 1/2] can: xilinx: Fix ksoftirq CPU monopolization
From: Matthias Auchmann
When using both the RXOK and the RXNEMP interrupt, when there were more
than one receive messages in the FIFO, ksoftirqd started to go crazy
and monopolize one CPU. The reason being is that RXOK just fires once
even if there are multiple frames sitting in the RX FIFO, so the code
didn't reflect that properly.
Changed that to only use the RXNEMP interrupt and thereby got rid of the
bug.
Signed-off-by: Matthias Auchmann
Acked-by: Kedareswara rao Appana
Signed-off-by: Michal Simek
---
drivers/net/can/xilinx_can.c | 18 +-
1 file changed, 5 insertions(+), 13 deletions(-)
diff --git a/drivers/net/can/xilinx_can.c b/drivers/net/can/xilinx_can.c
index 89aec07c225f..05ea2820d27b 100644
--- a/drivers/net/can/xilinx_can.c
+++ b/drivers/net/can/xilinx_can.c
@@ -101,7 +101,7 @@ enum xcan_reg {
#define XCAN_INTR_ALL (XCAN_IXR_TXOK_MASK | XCAN_IXR_BSOFF_MASK |\
XCAN_IXR_WKUP_MASK | XCAN_IXR_SLP_MASK | \
XCAN_IXR_RXNEMP_MASK | XCAN_IXR_ERROR_MASK | \
-XCAN_IXR_ARBLST_MASK | XCAN_IXR_RXOK_MASK)
+XCAN_IXR_ARBLST_MASK)
/* CAN register bit shift - XCAN___SHIFT */
#define XCAN_BTR_SJW_SHIFT 7 /* Synchronous jump width */
@@ -709,15 +709,7 @@ static int xcan_rx_poll(struct napi_struct *napi, int
quota)
isr = priv->read_reg(priv, XCAN_ISR_OFFSET);
while ((isr & XCAN_IXR_RXNEMP_MASK) && (work_done < quota)) {
- if (isr & XCAN_IXR_RXOK_MASK) {
- priv->write_reg(priv, XCAN_ICR_OFFSET,
- XCAN_IXR_RXOK_MASK);
- work_done += xcan_rx(ndev);
- } else {
- priv->write_reg(priv, XCAN_ICR_OFFSET,
- XCAN_IXR_RXNEMP_MASK);
- break;
- }
+ work_done += xcan_rx(ndev);
priv->write_reg(priv, XCAN_ICR_OFFSET, XCAN_IXR_RXNEMP_MASK);
isr = priv->read_reg(priv, XCAN_ISR_OFFSET);
}
@@ -728,7 +720,7 @@ static int xcan_rx_poll(struct napi_struct *napi, int quota)
if (work_done < quota) {
napi_complete_done(napi, work_done);
ier = priv->read_reg(priv, XCAN_IER_OFFSET);
- ier |= (XCAN_IXR_RXOK_MASK | XCAN_IXR_RXNEMP_MASK);
+ ier |= XCAN_IXR_RXNEMP_MASK;
priv->write_reg(priv, XCAN_IER_OFFSET, ier);
}
return work_done;
@@ -800,9 +792,9 @@ static irqreturn_t xcan_interrupt(int irq, void *dev_id)
}
/* Check for the type of receive interrupt and Processing it */
- if (isr & (XCAN_IXR_RXNEMP_MASK | XCAN_IXR_RXOK_MASK)) {
+ if (isr & XCAN_IXR_RXNEMP_MASK) {
ier = priv->read_reg(priv, XCAN_IER_OFFSET);
- ier &= ~(XCAN_IXR_RXNEMP_MASK | XCAN_IXR_RXOK_MASK);
+ ier &= ~XCAN_IXR_RXNEMP_MASK;
priv->write_reg(priv, XCAN_IER_OFFSET, ier);
napi_schedule(&priv->napi);
}
--
1.9.1
[PATCH 2/2] can: xilinx: fix RX FIFO overflow error handling
From: Andrea Scian
Simply resetting the peripheral on RX FIFO overflow in not enough,
because we also need to re-initialize the whole device.
Also always enable RX FIFO overflow interrupt otherwise we may hang
until another interrupt arrives (this happens if FIFO overrun and just
read from CAN bus with candump)
Signed-off-by: Andrea Scian
Reviewed-by: Kedareswara rao Appana
Signed-off-by: Michal Simek
---
drivers/net/can/xilinx_can.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/net/can/xilinx_can.c b/drivers/net/can/xilinx_can.c
index 05ea2820d27b..2c119d16861e 100644
--- a/drivers/net/can/xilinx_can.c
+++ b/drivers/net/can/xilinx_can.c
@@ -101,6 +101,7 @@ enum xcan_reg {
#define XCAN_INTR_ALL (XCAN_IXR_TXOK_MASK | XCAN_IXR_BSOFF_MASK |\
XCAN_IXR_WKUP_MASK | XCAN_IXR_SLP_MASK | \
XCAN_IXR_RXNEMP_MASK | XCAN_IXR_ERROR_MASK | \
+XCAN_IXR_RXOFLW_MASK | \
XCAN_IXR_ARBLST_MASK)
/* CAN register bit shift - XCAN___SHIFT */
@@ -529,6 +530,8 @@ static int xcan_rx(struct net_device *ndev)
return 1;
}
+static void xcan_chip_stop(struct net_device *ndev);
+
/**
* xcan_err_interrupt - error frame Isr
* @ndev: net_device pointer
@@ -600,7 +603,8 @@ static void xcan_err_interrupt(struct net_device *ndev, u32
isr)
if (isr & XCAN_IXR_RXOFLW_MASK) {
stats->rx_over_errors++;
stats->rx_errors++;
- priv->write_reg(priv, XCAN_SRR_OFFSET, XCAN_SRR_RESET_MASK);
+ xcan_chip_stop(ndev);
+ xcan_chip_start(ndev);
if (skb) {
cf->can_id |= CAN_ERR_CRTL;
cf->data[1] |= CAN_ERR_CRTL_RX_OVERFLOW;
--
1.9.1
Re: [PATCH] net: axienet: Remove unused parameter from __axienet_device_reset
On 13.10.2016 13:28, Tobias Klauser wrote:
> The dev parameter passed to __axienet_device_reset() is not used inside
> the function, so remove it.
>
> Signed-off-by: Tobias Klauser
> ---
> drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 11 +--
> 1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> index 35f9f9742a48..c688d68c39aa 100644
> --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> @@ -431,8 +431,7 @@ static void axienet_setoptions(struct net_device *ndev,
> u32 options)
> lp->options |= options;
> }
>
> -static void __axienet_device_reset(struct axienet_local *lp,
> -struct device *dev, off_t offset)
> +static void __axienet_device_reset(struct axienet_local *lp, off_t offset)
> {
> u32 timeout;
> /* Reset Axi DMA. This would reset Axi Ethernet core as well. The reset
> @@ -468,8 +467,8 @@ static void axienet_device_reset(struct net_device *ndev)
> u32 axienet_status;
> struct axienet_local *lp = netdev_priv(ndev);
>
> - __axienet_device_reset(lp, &ndev->dev, XAXIDMA_TX_CR_OFFSET);
> - __axienet_device_reset(lp, &ndev->dev, XAXIDMA_RX_CR_OFFSET);
> + __axienet_device_reset(lp, XAXIDMA_TX_CR_OFFSET);
> + __axienet_device_reset(lp, XAXIDMA_RX_CR_OFFSET);
>
> lp->max_frm_size = XAE_MAX_VLAN_FRAME_SIZE;
> lp->options |= XAE_OPTION_VLAN;
> @@ -1338,8 +1337,8 @@ static void axienet_dma_err_handler(unsigned long data)
> axienet_iow(lp, XAE_MDIO_MC_OFFSET, (mdio_mcreg &
> ~XAE_MDIO_MC_MDIOEN_MASK));
>
> - __axienet_device_reset(lp, &ndev->dev, XAXIDMA_TX_CR_OFFSET);
> - __axienet_device_reset(lp, &ndev->dev, XAXIDMA_RX_CR_OFFSET);
> + __axienet_device_reset(lp, XAXIDMA_TX_CR_OFFSET);
> + __axienet_device_reset(lp, XAXIDMA_RX_CR_OFFSET);
>
> axienet_iow(lp, XAE_MDIO_MC_OFFSET, mdio_mcreg);
> axienet_mdio_wait_until_ready(lp);
>
Can you please send this directly to mainline?
And put my:
Reviewed-by: Michal Simek
Thanks,
Michal
Re: [PATCH] net: axienet: Remove unused parameter from __axienet_device_reset
On 13.10.2016 16:40, Tobias Klauser wrote:
> On 2016-10-13 at 14:23:49 +0200, Michal Simek wrote:
>> On 13.10.2016 13:28, Tobias Klauser wrote:
>>> The dev parameter passed to __axienet_device_reset() is not used inside
>>> the function, so remove it.
>>>
>>> Signed-off-by: Tobias Klauser
>>> ---
>>> drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 11 +--
>>> 1 file changed, 5 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
>>> b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
>>> index 35f9f9742a48..c688d68c39aa 100644
>>> --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
>>> +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
>>> @@ -431,8 +431,7 @@ static void axienet_setoptions(struct net_device *ndev,
>>> u32 options)
>>> lp->options |= options;
>>> }
>>>
>>> -static void __axienet_device_reset(struct axienet_local *lp,
>>> - struct device *dev, off_t offset)
>>> +static void __axienet_device_reset(struct axienet_local *lp, off_t offset)
>>> {
>>> u32 timeout;
>>> /* Reset Axi DMA. This would reset Axi Ethernet core as well. The reset
>>> @@ -468,8 +467,8 @@ static void axienet_device_reset(struct net_device
>>> *ndev)
>>> u32 axienet_status;
>>> struct axienet_local *lp = netdev_priv(ndev);
>>>
>>> - __axienet_device_reset(lp, &ndev->dev, XAXIDMA_TX_CR_OFFSET);
>>> - __axienet_device_reset(lp, &ndev->dev, XAXIDMA_RX_CR_OFFSET);
>>> + __axienet_device_reset(lp, XAXIDMA_TX_CR_OFFSET);
>>> + __axienet_device_reset(lp, XAXIDMA_RX_CR_OFFSET);
>>>
>>> lp->max_frm_size = XAE_MAX_VLAN_FRAME_SIZE;
>>> lp->options |= XAE_OPTION_VLAN;
>>> @@ -1338,8 +1337,8 @@ static void axienet_dma_err_handler(unsigned long
>>> data)
>>> axienet_iow(lp, XAE_MDIO_MC_OFFSET, (mdio_mcreg &
>>> ~XAE_MDIO_MC_MDIOEN_MASK));
>>>
>>> - __axienet_device_reset(lp, &ndev->dev, XAXIDMA_TX_CR_OFFSET);
>>> - __axienet_device_reset(lp, &ndev->dev, XAXIDMA_RX_CR_OFFSET);
>>> + __axienet_device_reset(lp, XAXIDMA_TX_CR_OFFSET);
>>> + __axienet_device_reset(lp, XAXIDMA_RX_CR_OFFSET);
>>>
>>> axienet_iow(lp, XAE_MDIO_MC_OFFSET, mdio_mcreg);
>>> axienet_mdio_wait_until_ready(lp);
>>>
>>
>> Can you please send this directly to mainline?
>> And put my:
>> Reviewed-by: Michal Simek
>
> The netdev mailing list was Cc'ed on the patch, so it should end up in
> davem's patch queue.
ah ok. I missed that.
Then we are good.
Thanks,
Michal
Re: [Patch v4 01/12] microblaze: irqchip: Move intc driver to irqchip
On 2.9.2016 13:46, Zubair Lutfullah Kakakhel wrote: > Hi, > > Thanks for the valuable feedback. > Comments inline > > > On 09/02/2016 11:27 AM, Michal Simek wrote: >> On 2.9.2016 12:06, Zubair Lutfullah Kakakhel wrote: >>> Hi, >>> >>> On 09/02/2016 07:25 AM, Michal Simek wrote: >>>> On 1.9.2016 18:50, Zubair Lutfullah Kakakhel wrote: >>>>> The Xilinx AXI Interrupt Controller IP block is used by the MIPS >>>>> based xilfpga platform. >>>>> >>>>> Move the interrupt controller code out of arch/microblaze so that >>>>> it can be used by everyone >>>>> >>>>> Signed-off-by: Zubair Lutfullah Kakakhel >>>>> >>>>> --- >>>>> V3 -> V4 >>>>> No change >>>>> >>>>> V2 -> V3 >>>>> No change here. Cleanup patches follow after this patch. >>>>> Its debatable to cleanup before/after move. Decided to place cleanup >>>>> after move to put history in new place. >>>>> >>>>> V1 -> V2 >>>>> >>>>> Renamed irq-xilinx to irq-axi-intc >>>>> Renamed CONFIG_XILINX_INTC to CONFIG_XILINX_AXI_INTC >>>> >>>> >>>> I see that this was suggested by Jason Cooper but using axi name >>>> here is >>>> not correct. >>>> There is xps-intc name which is the name used on old OPB hardware >>>> designs. It means this driver can be still used only on system which >>>> uses it. >>> >>> Wouldn't axi-intc be more suitable moving forwards? >>> The IP block is now known as axi intc for 5 years as far as I can tell. >>> >>> Searching "axi intc" online results in the right docs for current and >>> future platforms. >> >> yes but we still should support older platform and it is more then this. >> This is soft-IP core and in future when there is new bus then IP will >> just change bus interface, etc. > > That makes sense. I'll rename the driver to irq-xps-intc.c > and CONFIG_XILINX_XPS_INTC > > Please shout now if anybody has issues with this. XPS was shortcut for design tools. You had CONFIG_XILINX_INTC which is IMHO the best name you can have. Thanks, Michal
Re: [Patch v4 01/12] microblaze: irqchip: Move intc driver to irqchip
On 2.9.2016 12:06, Zubair Lutfullah Kakakhel wrote: > Hi, > > On 09/02/2016 07:25 AM, Michal Simek wrote: >> On 1.9.2016 18:50, Zubair Lutfullah Kakakhel wrote: >>> The Xilinx AXI Interrupt Controller IP block is used by the MIPS >>> based xilfpga platform. >>> >>> Move the interrupt controller code out of arch/microblaze so that >>> it can be used by everyone >>> >>> Signed-off-by: Zubair Lutfullah Kakakhel >>> >>> --- >>> V3 -> V4 >>> No change >>> >>> V2 -> V3 >>> No change here. Cleanup patches follow after this patch. >>> Its debatable to cleanup before/after move. Decided to place cleanup >>> after move to put history in new place. >>> >>> V1 -> V2 >>> >>> Renamed irq-xilinx to irq-axi-intc >>> Renamed CONFIG_XILINX_INTC to CONFIG_XILINX_AXI_INTC >> >> >> I see that this was suggested by Jason Cooper but using axi name here is >> not correct. >> There is xps-intc name which is the name used on old OPB hardware >> designs. It means this driver can be still used only on system which >> uses it. > > Wouldn't axi-intc be more suitable moving forwards? > The IP block is now known as axi intc for 5 years as far as I can tell. > > Searching "axi intc" online results in the right docs for current and > future platforms. yes but we still should support older platform and it is more then this. This is soft-IP core and in future when there is new bus then IP will just change bus interface, etc. > > The binding is still xps-intc as that won't change. So older systems > should still be able to find their way. yes that's not a problem. But in general having bus name in name is not a good way to go. > >> Also there is another copy of this driver in the tree which was using >> old ppc405 and ppc440 xilinx platforms. >> >> arch/powerpc/include/asm/xilinx_intc.h >> arch/powerpc/sysdev/xilinx_intc.c >> >> These should be also removed by moving this driver to generic folder. > > I didn't know about that drivers existence. > > This patch series already touches microblaze, mips and irqchip. > Both microblaze and mips platforms using this driver are little-endian. MB is big ending too and as you see there is big endian support in the driver already. > > Adding a big-endian powerpc driver + platform to the mix is going to > complicate > the series further and make it super hard to synchronize various > subsystems, > test stuff, and then move the drivers without breakage. > > I'd highly recommend letting this move happen. And then the powerpc > driver can > transition over time to this driver. I have no problem with this but you should be aware about it. PPC will remain to use big endiand PLB bus. It means it is not axi too. Thanks, Michal
Re: [Patch v4 03/12] irqchip: axi-intc: Rename get_irq to xintc_get_irq
On 1.9.2016 18:50, Zubair Lutfullah Kakakhel wrote:
> Now that the driver is generic and used by multiple archs,
> get_irq is too generic.
>
> Rename get_irq to xintc_get_irq to avoid any conflicts
>
> Signed-off-by: Zubair Lutfullah Kakakhel
>
> ---
> V3 -> V4
> New patch.
> ---
> arch/microblaze/include/asm/irq.h | 2 +-
> arch/microblaze/kernel/irq.c | 4 ++--
> drivers/irqchip/irq-axi-intc.c| 4 ++--
> 3 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/arch/microblaze/include/asm/irq.h
> b/arch/microblaze/include/asm/irq.h
> index bab3b13..d785def 100644
> --- a/arch/microblaze/include/asm/irq.h
> +++ b/arch/microblaze/include/asm/irq.h
> @@ -16,6 +16,6 @@ struct pt_regs;
> extern void do_IRQ(struct pt_regs *regs);
>
> /* should be defined in each interrupt controller driver */
> -extern unsigned int get_irq(void);
> +extern unsigned int xintc_get_irq(void);
>
> #endif /* _ASM_MICROBLAZE_IRQ_H */
> diff --git a/arch/microblaze/kernel/irq.c b/arch/microblaze/kernel/irq.c
> index 11e24de..903dad8 100644
> --- a/arch/microblaze/kernel/irq.c
> +++ b/arch/microblaze/kernel/irq.c
> @@ -29,12 +29,12 @@ void __irq_entry do_IRQ(struct pt_regs *regs)
> trace_hardirqs_off();
>
> irq_enter();
> - irq = get_irq();
> + irq = xintc_get_irq();
> next_irq:
> BUG_ON(!irq);
> generic_handle_irq(irq);
>
> - irq = get_irq();
> + irq = xintc_get_irq();
> if (irq != -1U) {
> pr_debug("next irq: %d\n", irq);
> ++concurrent_irq;
> diff --git a/drivers/irqchip/irq-axi-intc.c b/drivers/irqchip/irq-axi-intc.c
> index 7ab0762..ac31548 100644
> --- a/drivers/irqchip/irq-axi-intc.c
> +++ b/drivers/irqchip/irq-axi-intc.c
> @@ -119,7 +119,7 @@ static struct irq_chip intc_dev = {
>
> static struct irq_domain *root_domain;
>
> -unsigned int get_irq(void)
> +unsigned int xintc_get_irq(void)
> {
> unsigned int hwirq, irq = -1;
>
> @@ -127,7 +127,7 @@ unsigned int get_irq(void)
> if (hwirq != -1U)
> irq = irq_find_mapping(root_domain, hwirq);
>
> - pr_debug("get_irq: hwirq=%d, irq=%d\n", hwirq, irq);
> + pr_debug("xintc_get_irq: hwirq=%d, irq=%d\n", hwirq, irq);
%s and __func__ instead.
Thanks,
Michal
Re: [Patch v4 09/12] net: ethernet: xilinx: Generate random mac if none found
On 1.9.2016 18:51, Zubair Lutfullah Kakakhel wrote:
> At the moment, if the emaclite device doesn't find a mac address
> from any source, it simply uses 0x0 with a warning printed.
>
> Instead of using a 0x0 mac address, use a randomly generated one.
>
> Signed-off-by: Zubair Lutfullah Kakakhel
>
> ---
> V3 -> V4
> Curly braces after if check for correct styling
>
> V2 -> V3
> No change
>
> V1 -> V2
> New patch
> ---
> drivers/net/ethernet/xilinx/xilinx_emaclite.c | 8 +---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/xilinx/xilinx_emaclite.c
> b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
> index 3cee84a..efc8d2e 100644
> --- a/drivers/net/ethernet/xilinx/xilinx_emaclite.c
> +++ b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
> @@ -1131,11 +1131,13 @@ static int xemaclite_of_probe(struct platform_device
> *ofdev)
> lp->rx_ping_pong = get_bool(ofdev, "xlnx,rx-ping-pong");
> mac_address = of_get_mac_address(ofdev->dev.of_node);
>
> - if (mac_address)
> + if (mac_address) {
> /* Set the MAC address. */
> memcpy(ndev->dev_addr, mac_address, ETH_ALEN);
> - else
> - dev_warn(dev, "No MAC address found\n");
> + } else {
> + dev_warn(dev, "No MAC address found. Generating Random one\n");
> + eth_hw_addr_random(ndev);
> + }
>
> /* Clear the Tx CSR's in case this is a restart */
> __raw_writel(0, lp->base_addr + XEL_TSR_OFFSET);
>
You should remove these 2 emaclite patches and send them separately
because they should go via netdev tree.
Thanks,
Michal
Re: [Patch v4 06/12] MIPS: xilfpga: Use Xilinx AXI Interrupt Controller
On 1.9.2016 18:50, Zubair Lutfullah Kakakhel wrote:
> IRQs from peripherals such as i2c/uart/ethernet come via
> the AXI Interrupt controller.
>
> Select it in Kconfig for xilfpga and add the DT node
>
> Signed-off-by: Zubair Lutfullah Kakakhel
>
> ---
> V3 -> V4
> No change
>
> V2 -> V3
> No change
>
> V1 -> V2
> Renamed select XILINX_INTC to select XILINX_AXI_INTC
> ---
> arch/mips/Kconfig| 1 +
> arch/mips/boot/dts/xilfpga/nexys4ddr.dts | 12
> 2 files changed, 13 insertions(+)
>
> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> index 2638856..e8a7786 100644
> --- a/arch/mips/Kconfig
> +++ b/arch/mips/Kconfig
> @@ -426,6 +426,7 @@ config MACH_XILFPGA
> select SYS_SUPPORTS_ZBOOT_UART16550
> select USE_OF
> select USE_GENERIC_EARLY_PRINTK_8250
> + select XILINX_AXI_INTC
> help
> This enables support for the IMG University Program MIPSfpga platform.
>
> diff --git a/arch/mips/boot/dts/xilfpga/nexys4ddr.dts
> b/arch/mips/boot/dts/xilfpga/nexys4ddr.dts
> index 48d2112..8db660b 100644
> --- a/arch/mips/boot/dts/xilfpga/nexys4ddr.dts
> +++ b/arch/mips/boot/dts/xilfpga/nexys4ddr.dts
> @@ -17,6 +17,18 @@
> compatible = "mti,cpu-interrupt-controller";
> };
>
> + axi_intc: interrupt-controller@1020 {
> + #interrupt-cells = <1>;
> + compatible = "xlnx,xps-intc-1.00.a";
> + interrupt-controller;
> + reg = <0x1020 0x1>;
> + xlnx,kind-of-intr = <0x0>;
> + xlnx,num-intr-inputs = <0x6>;
> +
> + interrupt-parent = <&cpuintc>;
> + interrupts = <6>;
this is not the part of binding that's why you should remove it.
number of inputs is above that's why this is duplication.
Thanks,
Michal
Re: [Patch v4 06/12] MIPS: xilfpga: Use Xilinx AXI Interrupt Controller
On 2.9.2016 09:05, Michal Simek wrote:
> On 1.9.2016 18:50, Zubair Lutfullah Kakakhel wrote:
>> IRQs from peripherals such as i2c/uart/ethernet come via
>> the AXI Interrupt controller.
>>
>> Select it in Kconfig for xilfpga and add the DT node
>>
>> Signed-off-by: Zubair Lutfullah Kakakhel
>>
>> ---
>> V3 -> V4
>> No change
>>
>> V2 -> V3
>> No change
>>
>> V1 -> V2
>> Renamed select XILINX_INTC to select XILINX_AXI_INTC
>> ---
>> arch/mips/Kconfig| 1 +
>> arch/mips/boot/dts/xilfpga/nexys4ddr.dts | 12
>> 2 files changed, 13 insertions(+)
>>
>> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
>> index 2638856..e8a7786 100644
>> --- a/arch/mips/Kconfig
>> +++ b/arch/mips/Kconfig
>> @@ -426,6 +426,7 @@ config MACH_XILFPGA
>> select SYS_SUPPORTS_ZBOOT_UART16550
>> select USE_OF
>> select USE_GENERIC_EARLY_PRINTK_8250
>> +select XILINX_AXI_INTC
>> help
>>This enables support for the IMG University Program MIPSfpga platform.
>>
>> diff --git a/arch/mips/boot/dts/xilfpga/nexys4ddr.dts
>> b/arch/mips/boot/dts/xilfpga/nexys4ddr.dts
>> index 48d2112..8db660b 100644
>> --- a/arch/mips/boot/dts/xilfpga/nexys4ddr.dts
>> +++ b/arch/mips/boot/dts/xilfpga/nexys4ddr.dts
>> @@ -17,6 +17,18 @@
>> compatible = "mti,cpu-interrupt-controller";
>> };
>>
>> +axi_intc: interrupt-controller@1020 {
>> +#interrupt-cells = <1>;
>> +compatible = "xlnx,xps-intc-1.00.a";
>> +interrupt-controller;
>> +reg = <0x1020 0x1>;
>> +xlnx,kind-of-intr = <0x0>;
>> +xlnx,num-intr-inputs = <0x6>;
>> +
>> +interrupt-parent = <&cpuintc>;
>> +interrupts = <6>;
>
> this is not the part of binding that's why you should remove it.
> number of inputs is above that's why this is duplication.
Sorry my bad - this 6 is not number of input but irq to primary
interrupt controller.
Thanks,
Michal
Re: [Patch v4 01/12] microblaze: irqchip: Move intc driver to irqchip
On 1.9.2016 18:50, Zubair Lutfullah Kakakhel wrote: > The Xilinx AXI Interrupt Controller IP block is used by the MIPS > based xilfpga platform. > > Move the interrupt controller code out of arch/microblaze so that > it can be used by everyone > > Signed-off-by: Zubair Lutfullah Kakakhel > > --- > V3 -> V4 > No change > > V2 -> V3 > No change here. Cleanup patches follow after this patch. > Its debatable to cleanup before/after move. Decided to place cleanup > after move to put history in new place. > > V1 -> V2 > > Renamed irq-xilinx to irq-axi-intc > Renamed CONFIG_XILINX_INTC to CONFIG_XILINX_AXI_INTC I see that this was suggested by Jason Cooper but using axi name here is not correct. There is xps-intc name which is the name used on old OPB hardware designs. It means this driver can be still used only on system which uses it. Also there is another copy of this driver in the tree which was using old ppc405 and ppc440 xilinx platforms. arch/powerpc/include/asm/xilinx_intc.h arch/powerpc/sysdev/xilinx_intc.c These should be also removed by moving this driver to generic folder. Thanks, Michal
Re: [Patch v4 01/12] microblaze: irqchip: Move intc driver to irqchip
On 1.9.2016 18:50, Zubair Lutfullah Kakakhel wrote: > The Xilinx AXI Interrupt Controller IP block is used by the MIPS > based xilfpga platform. > > Move the interrupt controller code out of arch/microblaze so that > it can be used by everyone if this is just move that you should setup your git right that it won't be remove on one side and add on other side. It should look like this when git mv is used and git setup is right. diff --git a/Makefile b/Makefile2 similarity index 100% rename from Makefile rename to Makefile2 Thanks, Michal
Re: [RFC PATCH 2/3] net: macb: Add support for 1588 for Zynq Ultrascale+ MPSoC
Hi Nicolas, just a note: Here is the link to public Linux repo https://github.com/Xilinx/linux-xlnx Thanks, Michal On 9.8.2016 18:56, Punnaiah Choudary Kalluri wrote: > Hi Nicolas, > > 1588 implementation in cadence GEM IP we have in Zynq Ultascale+ MPSoC is > Different to the one in Zynq SOC. > > In earlier version, all timestamp values will be stored in registers and > there is no specific > Mechanism to distinguish the received ethernet frame that contains time stamp > information > Other than parsing the frame for PTP packet type. > > We have basic implementation for earlier version in our out of tree driver, > which is going to be deprecated > Soon. You could also check the below driver for 1588 support. > https://gitenterprise.xilinx.com/Linux/linux-xlnx/blob/master/drivers/net/ethernet/xilinx/xilinx_emacps.c > > > Regards, > Punnaiah > >> -Original Message- >> From: Nicolas Ferre [mailto:nicolas.fe...@atmel.com] >> Sent: Tuesday, August 09, 2016 10:10 PM >> To: Harini Katakam ; Harini Katakam >> ; Andrei Pistirica >> Cc: da...@davemloft.net; Boris Brezillon > electrons.com>; alexandre.bell...@free-electrons.com; >> netdev@vger.kernel.org; linux-ker...@vger.kernel.org; >> devicet...@vger.kernel.org; Punnaiah Choudary Kalluri >> ; Michal Simek ; Anirudha >> Sarangi >> Subject: Re: [RFC PATCH 2/3] net: macb: Add support for 1588 for Zynq >> Ultrascale+ MPSoC >> >> Le 21/09/2015 à 19:49, Harini Katakam a écrit : >>> On Fri, Sep 11, 2015 at 1:27 PM, Harini Katakam >>> wrote: >>>> Cadence GEM in Zynq Ultrascale+ MPSoC supports 1588 and provides a >>>> 102 bit time counter with 48 bits for seconds, 30 bits for nsecs and >>>> 24 bits for sub-nsecs. The timestamp is made available to the SW through >>>> registers as well as (more precisely) through upper two words in >>>> an extended BD. >>>> >>>> This patch does the following: >>>> - Adds MACB_CAPS_TSU in zynqmp_config. >>>> - Registers to ptp clock framework (after checking for timestamp support >> in >>>> IP and capability in config). >>>> - TX BD and RX BD control registers are written to populate timestamp in >>>> extended BD words. >>>> - Timer initialization is done by writing time of day to the timer counter. >>>> - ns increment register is programmed as NS_PER_SEC/TSU_CLK. >>>> For a 24 bit subns precision, the subns increment equals >>>> remainder of (NS_PER_SEC/TSU_CLK) * (2^24). >>>> TSU (Time stamp unit) clock is obtained by the driver from devicetree. >>>> - HW time stamp capabilities are advertised via ethtool and macb ioctl is >>>> updated accordingly. >>>> - For all PTP event frames, nanoseconds and the lower 5 bits of seconds >> are >>>> obtained from the BD. This offers a precise timestamp. The upper bits >>>> (which dont vary between consecutive packets) are obtained from the >>>> TX/RX PTP event/PEER registers. The timestamp obtained thus is >> updated >>>> in skb for upper layers to access. >>>> - The drivers register functions with ptp to perform time and frequency >>>> adjustment. >>>> - Time adjustment is done by writing to the 1558_ADJUST register. >>>> The controller will read the delta in this register and update the timer >>>> counter register. Alternatively, for large time offset adjustments, >>>> the driver reads the secs and nsecs counter values, adds/subtracts the >>>> delta and updates the timer counter. In order to be as precise as >> possible, >>>> nsecs counter is read again if secs has incremented during the counter >> read. >>>> - Frequency adjustment is not directly supported by this IP. >>>> addend is the initial value ns increment and similarly addendesub. >>>> The ppb (parts per billion) provided is used as >>>> ns_incr = addend +/- (ppb/rate). >>>> Similarly the remainder of the above is used to populate subns >> increment. >>>> In case the ppb requested is negative AND subns adjustment greater >> than >>>> the addendsub, ns_incr is reduced by 1 and subns_incr is adjusted in >>>> positive accordingly. >>>> >>>> Signed-off-by: Harini Katakam : >>>> --- >>>> drivers/net/ethernet/cadence/macb.c | 372 >> ++- >>>> drivers/net/ethernet/cadence/macb.h | 64 ++ >>
Re: [RFC PATCH v4 1/2] Documentation: DT: net: Add Xilinx gmiitorgmii converter device tree binding documentation
On 8.8.2016 09:15, Kedareswara rao Appana wrote:
> Device-tree binding documentation for xilinx gmiitorgmii converter.
>
> Signed-off-by: Kedareswara rao Appana
> ---
> Changes for v4:
> --> Modified compatible as suggested by Rob.
> --> Removed underscores from the converter node name as suggested by Rob.
> Changes for v3:
> --> None.
> Changes for v2:
> --> New patch.
>
> .../devicetree/bindings/net/xilinx_gmii2rgmii.txt | 38
> ++
> 1 file changed, 38 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/net/xilinx_gmii2rgmii.txt
>
> diff --git a/Documentation/devicetree/bindings/net/xilinx_gmii2rgmii.txt
> b/Documentation/devicetree/bindings/net/xilinx_gmii2rgmii.txt
> new file mode 100644
> index 000..453680d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/xilinx_gmii2rgmii.txt
> @@ -0,0 +1,38 @@
> +XILINX GMIITORGMII Converter Driver Device Tree Bindings
> +
> +
> +The Gigabit Media Independent Interface (GMII) to Reduced Gigabit Media
> +Independent Interface (RGMII) core provides the RGMII between RGMII-compliant
> +Ethernet physical media devices (PHY) and the Gigabit Ethernet controller.
> +This core can be used in all three modes of operation(10/100/1000 Mb/s).
> +The Management Data Input/Output (MDIO) interface is used to configure the
> +Speed of operation. This core can switch dynamically between the three
> +Different speed modes by configuring the conveter register through mdio
> write.
> +
> +The MDIO is a bus to which the PHY devices are connected. For each
> +device that exists on this bus, a child node should be created. See
> +the definition of the PHY node in booting-without-of.txt for an example
> +of how to define a PHY.
> +
> +This converter sits between the MAC and the external phy.
> +MAC <==> GMII2RGMII <==> RGMII_PHY
> +
> +Required properties:
> + - compatible : Should be "xlnx,gmii-to-rgmii-1.0"
> + - reg : The ID number for the phy, usually a small integer
> + - phy-handle: Should point to the external phy device.
> + See ethernet.txt file in the same directory.
> +
> +Example:
> + mdio {
> +#address-cells = <1>;
> +#size-cells = <0>;
> + phy: ethernet-phy@0 {
> + ..
> + };
> +gmiitorgmii: gmiitorgmii@8 {
> +compatible = "xlnx,gmii-to-rgmii-1.0";
> +reg = <8>;
> + phy-handle = <&phy>;
> +};
Indentation in this example is quite weird. You are mixing tabs and spaces.
Thanks,
Michal
Re: [PATCH 5/5] net: macb: Fix simple typo.
On 13.3.2016 20:10, Moritz Fischer wrote: > Signed-off-by: Moritz Fischer > --- > drivers/net/ethernet/cadence/macb.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/cadence/macb.c > b/drivers/net/ethernet/cadence/macb.c > index a0c01e5..681e5bf 100644 > --- a/drivers/net/ethernet/cadence/macb.c > +++ b/drivers/net/ethernet/cadence/macb.c > @@ -127,7 +127,7 @@ static void hw_writel(struct macb *bp, int offset, u32 > value) > } > > /* Find the CPU endianness by using the loopback bit of NCR register. When > the > - * CPU is in big endian we need to program swaped mode for management > + * CPU is in big endian we need to program swapped mode for management > * descriptor access. > */ > static bool hw_is_native_io(void __iomem *addr) > Remove dot at the end of subject and feel free to add my: Acked-by: Michal Simek Thanks, Michal
Re: [PATCH 4/5] net: macb: Use ether_addr_copy over memcpy
On 13.3.2016 20:10, Moritz Fischer wrote: > Checkpatch suggests using ether_addr_copy over memcpy > to copy the mac address. > > Signed-off-by: Moritz Fischer > --- > drivers/net/ethernet/cadence/macb.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/cadence/macb.c > b/drivers/net/ethernet/cadence/macb.c > index 53400f6..a0c01e5 100644 > --- a/drivers/net/ethernet/cadence/macb.c > +++ b/drivers/net/ethernet/cadence/macb.c > @@ -2891,7 +2891,7 @@ static int macb_probe(struct platform_device *pdev) > > mac = of_get_mac_address(np); > if (mac) > - memcpy(bp->dev->dev_addr, mac, ETH_ALEN); > + ether_addr_copy(bp->dev->dev_addr, mac); > else > macb_get_hwaddr(bp); > > Acked-by: Michal Simek M
Re: [PATCH 1/5] net: macb: Fix coding style error message
On 13.3.2016 20:10, Moritz Fischer wrote:
> checkpatch.pl gave the following error:
>
> ERROR: space required before the open parenthesis '('
> + for(; p < end; p++, offset += 4)
>
> Signed-off-by: Moritz Fischer
> ---
> drivers/net/ethernet/cadence/macb.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/cadence/macb.c
> b/drivers/net/ethernet/cadence/macb.c
> index 50c9410..4370f37 100644
> --- a/drivers/net/ethernet/cadence/macb.c
> +++ b/drivers/net/ethernet/cadence/macb.c
> @@ -496,7 +496,7 @@ static void macb_update_stats(struct macb *bp)
>
> WARN_ON((unsigned long)(end - p - 1) != (MACB_TPF - MACB_PFR) / 4);
>
> - for(; p < end; p++, offset += 4)
> + for (; p < end; p++, offset += 4)
> *p += bp->macb_reg_readl(bp, offset);
> }
>
>
Acked-by: Michal Simek
Thanks,
Michal
Re: [PATCH 2/5] net: macb: Fix coding style warnings
On 13.3.2016 20:10, Moritz Fischer wrote: > This commit takes care of the coding style warnings > that are mostly due to a different comment style and > lines over 80 chars, as well as a dangling else. > > Signed-off-by: Moritz Fischer > --- > drivers/net/ethernet/cadence/macb.c | 101 > +++- > 1 file changed, 43 insertions(+), 58 deletions(-) > > diff --git a/drivers/net/ethernet/cadence/macb.c > b/drivers/net/ethernet/cadence/macb.c > index 4370f37..c2d31c5 100644 > --- a/drivers/net/ethernet/cadence/macb.c > +++ b/drivers/net/ethernet/cadence/macb.c > @@ -58,8 +58,7 @@ > > #define GEM_MTU_MIN_SIZE 68 > > -/* > - * Graceful stop timeouts in us. We should allow up to > +/* Graceful stop timeouts in us. We should allow up to > * 1 frame time (10 Mbits/s, full-duplex, ignoring collisions) > */ > #define MACB_HALT_TIMEOUT1230 > @@ -127,8 +126,7 @@ static void hw_writel(struct macb *bp, int offset, u32 > value) > writel_relaxed(value, bp->regs + offset); > } > > -/* > - * Find the CPU endianness by using the loopback bit of NCR register. When > the > +/* Find the CPU endianness by using the loopback bit of NCR register. When > the TBH: I would rather see this converting to kernel-doc format instead of using this networking block. Also splitting this to more patches will be better. Just by categories but that's just my opinion. Thanks, Michal
Re: [PATCH 3/5] net: macb: Address checkpatch 'check' suggestions
frag_len);
> offset += bp->rx_buffer_size;
> desc = macb_rx_desc(bp, frag);
> desc->addr &= ~MACB_BIT(RX_USED);
> @@ -934,7 +935,7 @@ static int macb_rx_frame(struct macb *bp, unsigned int
> first_frag,
> bp->stats.rx_packets++;
> bp->stats.rx_bytes += skb->len;
> netdev_vdbg(bp->dev, "received skb of length %u, csum: %08x\n",
> -skb->len, skb->csum);
> + skb->len, skb->csum);
> netif_receive_skb(skb);
>
> return 0;
> @@ -999,7 +1000,7 @@ static int macb_poll(struct napi_struct *napi, int
> budget)
> work_done = 0;
>
> netdev_vdbg(bp->dev, "poll: status = %08lx, budget = %d\n",
> -(unsigned long)status, budget);
> + (unsigned long)status, budget);
>
> work_done = bp->macbgem_ops.mog_rx(bp, budget);
> if (work_done < budget) {
> @@ -1214,7 +1215,7 @@ static unsigned int macb_tx_map(struct macb *bp,
> }
>
> /* Should never happen */
> - if (unlikely(tx_skb == NULL)) {
> + if (unlikely(!tx_skb)) {
> netdev_err(bp->dev, "BUG! empty skb!\n");
> return 0;
> }
> @@ -1284,16 +1285,16 @@ static int macb_start_xmit(struct sk_buff *skb,
> struct net_device *dev)
>
> #if defined(DEBUG) && defined(VERBOSE_DEBUG)
> netdev_vdbg(bp->dev,
> -"start_xmit: queue %hu len %u head %p data %p tail %p end
> %p\n",
> -queue_index, skb->len, skb->head, skb->data,
> -skb_tail_pointer(skb), skb_end_pointer(skb));
> + "start_xmit: queue %hu len %u head %p data %p tail %p end
> %p\n",
> + queue_index, skb->len, skb->head, skb->data,
> + skb_tail_pointer(skb), skb_end_pointer(skb));
> print_hex_dump(KERN_DEBUG, "data: ", DUMP_PREFIX_OFFSET, 16, 1,
> skb->data, 16, true);
> #endif
>
> /* Count how many TX buffer descriptors are needed to send this
>* socket buffer: skb fragments of jumbo frames may need to be
> - * splitted into many buffer descriptors.
> + * split into many buffer descriptors.
>*/
> count = DIV_ROUND_UP(skb_headlen(skb), bp->max_tx_length);
> nr_frags = skb_shinfo(skb)->nr_frags;
> @@ -1344,8 +1345,8 @@ static void macb_init_rx_buffer_size(struct macb *bp,
> size_t size)
>
> if (bp->rx_buffer_size % RX_BUFFER_MULTIPLE) {
> netdev_dbg(bp->dev,
> - "RX buffer must be multiple of %d bytes,
> expanding\n",
> - RX_BUFFER_MULTIPLE);
> +"RX buffer must be multiple of %d bytes,
> expanding\n",
> +RX_BUFFER_MULTIPLE);
> bp->rx_buffer_size =
> roundup(bp->rx_buffer_size, RX_BUFFER_MULTIPLE);
> }
> @@ -1368,7 +1369,7 @@ static void gem_free_rx_buffers(struct macb *bp)
> for (i = 0; i < RX_RING_SIZE; i++) {
> skb = bp->rx_skbuff[i];
>
> - if (skb == NULL)
> + if (!skb)
> continue;
>
> desc = &bp->rx_ring[i];
> @@ -1776,7 +1777,8 @@ static void macb_sethashtable(struct net_device *dev)
> unsigned int bitnr;
> struct macb *bp = netdev_priv(dev);
>
> - mc_filter[0] = mc_filter[1] = 0;
> + mc_filter[0] = 0;
> + mc_filter[1] = 0;
>
> netdev_for_each_mc_addr(ha, dev) {
> bitnr = hash_get_index(ha->addr);
>
Acked-by: Michal Simek
M
Re: [PATCH 0/3] net: macb: Fix coding style issues
On 9.3.2016 18:22, David Miller wrote: > From: Michal Simek > Date: Wed, 9 Mar 2016 17:29:39 +0100 > >> On 7.3.2016 18:13, Nicolas Ferre wrote: >>> Le 07/03/2016 17:17, Moritz Fischer a écrit : >>>> Hi Nicolas, >>>> >>>> this series deals with most of the checkpatch warnings >>>> generated for macb. There are two BUG_ON()'s that I didn't touch, yet, >>>> that were suggested by checkpatch, that I can address in a follow up >>>> commit if needed. >>>> Let me know if you want me to split the fixes differently or squash >>>> them into one commit. >>> >>> Hi, >>> >>> I'm not usually fond of this type of patches, but I must admit that this >>> series corrects some style issues. >>> >>> So, I would like more feedback from Michal and Cyrille as these changes >>> may delay some of the not-merged-yet features or more important >>> work-in-progress on their side. >>> >>> On the other hand, if we all think it's a calm period for this macb >>> driver, we may find interesting to merge some "cleanup and style" >>> enhancements. >> >> Not a problem with merging cleanups in general. We have several out of >> tree patches but doesn't make sense to to wait. >> I wasn't in cc for the series but I don't like this change to be the >> part of cleanup series. >> >> mac = of_get_mac_address(np); >> if (mac) >> -memcpy(bp->dev->dev_addr, mac, ETH_ALEN); >> +ether_addr_copy(bp->dev->dev_addr, mac); > > Why? This is what we tell people to use. I would expect this as separate patch not the part of one huge cleanup patch which does just comment and space cleanups. Thanks, Michal
Re: [PATCH 0/3] net: macb: Fix coding style issues
On 7.3.2016 18:13, Nicolas Ferre wrote: > Le 07/03/2016 17:17, Moritz Fischer a écrit : >> Hi Nicolas, >> >> this series deals with most of the checkpatch warnings >> generated for macb. There are two BUG_ON()'s that I didn't touch, yet, >> that were suggested by checkpatch, that I can address in a follow up >> commit if needed. >> Let me know if you want me to split the fixes differently or squash >> them into one commit. > > Hi, > > I'm not usually fond of this type of patches, but I must admit that this > series corrects some style issues. > > So, I would like more feedback from Michal and Cyrille as these changes > may delay some of the not-merged-yet features or more important > work-in-progress on their side. > > On the other hand, if we all think it's a calm period for this macb > driver, we may find interesting to merge some "cleanup and style" > enhancements. Not a problem with merging cleanups in general. We have several out of tree patches but doesn't make sense to to wait. I wasn't in cc for the series but I don't like this change to be the part of cleanup series. mac = of_get_mac_address(np); if (mac) - memcpy(bp->dev->dev_addr, mac, ETH_ALEN); + ether_addr_copy(bp->dev->dev_addr, mac); else Also extending scope of variables is not the right way to go. Especially when some automation tools are reporting that you should reduce scope of use for them. Wolfram is checking it for example. Thanks, Michal
Re: [PATCH 1/2] can: xilinx: use readl/writel instead of ioread/iowrite
On 10/22/2015 11:02 AM, Arnd Bergmann wrote: > On Thursday 22 October 2015 08:34:53 Appana Durga Kedareswara Rao wrote: >>> On Thursday 22 October 2015 10:16:02 Kedareswara rao Appana wrote: The driver only supports memory-mapped I/O [by ioremap()], so readl/writel is actually the right thing to do, IMO. During the validation of this driver or IP on ARM 64-bit processor while sending lot of packets observed that the tx packet drop with iowrite Putting the barriers for each tx fifo register write fixes this issue Instead of barriers using writel also fixed this issue. Signed-off-by: Kedareswara rao Appana >>> >>> The two should really do the same thing: iowrite32() is just a static >>> inline calling >>> writel() on both ARM32 and ARM64. On which kernel version did you observe >>> the >>> difference? It's possible that an older version used CONFIG_GENERIC_IOMAP, >>> which made this slightly more expensive. >> >> I observed this issue with the 4.0.0 kernel version > > Is it possible that you have nonstandard patches on your kernel? If so, can > you send a diff against the mainline version? Kedar: Can you please retest this on mainline kernel? We shouldn't have any patches which should influence this. Thanks, Michal -- 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: [RFC PATCH 0/2] net: macb: Add mdio driver for accessing multiple phy devices
Hi, On 07/31/2015 11:53 PM, Nathan Sullivan wrote: > On Tue, Jul 28, 2015 at 03:34:51AM +, Punnaiah Choudary Kalluri wrote: >> Ok. I will send you updated patch for mdio support soon and we will finalize >> next >> Course of actions if it doesn't break the existing flow. >> >> Thanks, >> Punnaiah > > Just a heads up, when mdio no longer turns off when macb goes down, the micrel > 9031 phy will have an issue with interrupts getting disabling during phy > suspend. I have a patch to correct this issue here: > > https://patchwork.ozlabs.org/patch/502189/ > > Would you mind including this patch in your set? You should resend the patch again and you got one more argument why this patch should go it. But it should go in own direction out of this patch. Thanks, Michal -- 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: [RFC PATCH 0/2] net: macb: Add mdio driver for accessing multiple phy devices
Hi Nicolas, have you had a time to look at this? Thanks, Michal On 07/13/2015 06:48 AM, Punnaiah Choudary Kalluri wrote: > This patch is to add support for the design that has multiple ethernet > mac controllers and single mdio bus connected to multiple phy devices. > i.e mdio lines are connected to any of the ethernet mac controller and > all the phy devices will be accessed using the phy maintenance interface > in that mac controller. > > __ _ > | | |PHY0 | > | MAC0 |-| | > |__| | |_| >| > __| _ > | | | | | > | MAC1 | |_|PHY1 | > |__| | | > > So, i come up with two implementations for addressing the above configuration. > > Implementation 1: > Have separate driver for mdio bus > Create a DT node for all the PHY devices connected to the mdio bus > This driver will share the register space of the mac controller that has > mdio bus connected. > > Implementation 2: > Add new property "has-mdio" and it should be 1 for the mac that has mdio bus > connected. > Create the mdio bus only when the has-mdio property is 1 > > Please review the two implementations and suggest which one is better to > proceed > further. In my opinion implementation 1 will be the ideal one. > > Currently i have tested the patches with single mac and single phy > configuration. I need to take care of few more cases before releasing the > final patch > but before that i would like to have your opinion on the above implementations > and finalize one implementation. so that i can enhance it further. > > Punnaiah Choudary Kalluri (1): > net: macb: Add mdio driver for accessing multiple phy devices > net: macb: Add support for single mac managing more than one phy > > > drivers/net/ethernet/cadence/Makefile|2 +- > drivers/net/ethernet/cadence/macb.c | 93 +- > drivers/net/ethernet/cadence/macb.h |3 +- > drivers/net/ethernet/cadence/macb_mdio.c | 204 > ++ > 4 files changed, 211 insertions(+), 91 deletions(-) > create mode 100644 drivers/net/ethernet/cadence/macb_mdio.c > -- 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: ll_temac: Remove sparse warnings
Remove sparse warnings:
drivers/net/ethernet/xilinx/ll_temac_main.c:65:16: warning: cast removes
address space of expression
drivers/net/ethernet/xilinx/ll_temac_main.c:70:9: warning: cast removes
address space of expression
drivers/net/ethernet/xilinx/ll_temac_main.c:127:16: warning: cast
removes address space of expression
drivers/net/ethernet/xilinx/ll_temac_main.c:137:9: warning: cast removes
address space of expression
drivers/net/ethernet/xilinx/ll_temac_main.c:409:3: warning: symbol
'temac_options' was not declared. Should it be static?
drivers/net/ethernet/xilinx/ll_temac_main.c:590:6: warning: symbol
'temac_adjust_link' was not declared. Should it be static?
Signed-off-by: Michal Simek
---
drivers/net/ethernet/xilinx/ll_temac_main.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/xilinx/ll_temac_main.c
b/drivers/net/ethernet/xilinx/ll_temac_main.c
index 3b99a4df71f8..5a1068df7038 100644
--- a/drivers/net/ethernet/xilinx/ll_temac_main.c
+++ b/drivers/net/ethernet/xilinx/ll_temac_main.c
@@ -62,12 +62,12 @@
u32 temac_ior(struct temac_local *lp, int offset)
{
- return in_be32((u32 *)(lp->regs + offset));
+ return in_be32(lp->regs + offset);
}
void temac_iow(struct temac_local *lp, int offset, u32 value)
{
- out_be32((u32 *) (lp->regs + offset), value);
+ out_be32(lp->regs + offset, value);
}
int temac_indirect_busywait(struct temac_local *lp)
@@ -124,7 +124,7 @@ void temac_indirect_out32(struct temac_local *lp, int reg,
u32 value)
*/
static u32 temac_dma_in32(struct temac_local *lp, int reg)
{
- return in_be32((u32 *)(lp->sdma_regs + (reg << 2)));
+ return in_be32(lp->sdma_regs + (reg << 2));
}
/**
@@ -134,7 +134,7 @@ static u32 temac_dma_in32(struct temac_local *lp, int reg)
*/
static void temac_dma_out32(struct temac_local *lp, int reg, u32 value)
{
- out_be32((u32 *)(lp->sdma_regs + (reg << 2)), value);
+ out_be32(lp->sdma_regs + (reg << 2), value);
}
/* DMA register access functions can be DCR based or memory mapped.
@@ -400,7 +400,7 @@ static void temac_set_multicast_list(struct net_device
*ndev)
mutex_unlock(&lp->indirect_mutex);
}
-struct temac_option {
+static struct temac_option {
int flg;
u32 opt;
u32 reg;
@@ -587,7 +587,7 @@ static void temac_device_reset(struct net_device *ndev)
ndev->trans_start = jiffies; /* prevent tx timeout */
}
-void temac_adjust_link(struct net_device *ndev)
+static void temac_adjust_link(struct net_device *ndev)
{
struct temac_local *lp = netdev_priv(ndev);
struct phy_device *phy = lp->phy_dev;
--
2.3.5
--
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: ll_temac: Use one return statement instead of two
On 05/07/2015 05:30 PM, Julia Lawall wrote: > On Thu, 7 May 2015, Michal Simek wrote: > >> From: Michal Simek >> >> Use one return statement instead of two to simplify the code. >> Both are returning the same value. >> >> Signed-off-by: Michal Simek > > The from should be the same as the signed off. You need From if you want > something different than what is naturally put by your mailer. But that > doesn't see to be the case. ok. Fixed in v2. Thanks, Michal -- 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 02/12] net: axienet: Handle 0 packet receive gracefully
On 05/05/2015 03:57 PM, Joe Perches wrote: > On Tue, 2015-05-05 at 11:25 +0200, Michal Simek wrote: >> From: Peter Crosthwaite >> >> The AXI-DMA rx-delay interrupt can sometimes be triggered >> when there are 0 outstanding packets received. This is due >> to the fact that the receive function will greedily consume >> as many packets as possible on interrupt. So if two packets >> (with a very particular timing) arrive in succession they >> will each cause the rx-delay interrupt, but the first interrupt >> will consume both packets. >> This means the second interrupt is a 0 packet receive. >> >> This is mostly OK, except that the tail pointer register is >> updated unconditionally on receive. Currently the tail pointer >> is always set to the current bd-ring descriptor under >> the assumption that the hardware has moved onto the next >> descriptor. What this means for length 0 recv is the current >> descriptor that the hardware is potentially yet to use will >> be marked as the tail. This causes the hardware to think >> its run out of descriptors deadlocking the whole rx path. >> >> Fixed by updating the tail pointer to the most recent >> successfully consumed descriptor. > > I think some of this would be good to have as comments > in the code instead of just in the changelog. Is it really needed? If yes, no problem to add it but git blame can point you to that. Thanks, Michal -- 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
