Re: stmmac: GMAC_RGSMIIIS reports bogus values

[Vineet Gupta]

On 01/25/2017 10:39 AM, Alexey Brodkin wrote:
>> Also I wonder if, other version of the stmmac worked on this platform
>> before.
> It did work and still works. The only problem is we're getting
> a lot of noise now about bogus link status change. That's because
> this info is now in pr_info() compared to being previously in pr_debug().

While we sort out the real technical details, it is OK to go back to pr_debug
please - print_once or some such !
There is lot of useless console noise when we run any networking loads here !

echo 0 >  /proc/sys/kernel/printk

only helps in that it is not printed on console but clobbers dmesg nonetheless !

Thx,
-Vineet

___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: stmmac: GMAC_RGSMIIIS reports bogus values

[Alexey Brodkin]

Hi Giuseppe,

On Mon, 2016-11-14 at 09:14 +0100, Giuseppe CAVALLARO wrote:
> Hello Alexey
> 
> Sorry for my late reply. In that case, I think that the stmmac
> is using own RGMII/SGMII support (PCS) to dialog with the
> transceiver. I think, from the HW capability register
> this feature is present and the driver is using it as default.

Yep looks like that.

> I kindly ask you to verify if this is your setup or if you
> do not want to use it. In that case, it is likely we need to
> add some code in the driver.

Well GMAC's databook says:
--->8--
The DWC_gmac provides an IEEE 802.3z compliant 10-bit
Physical Coding Sublayer (PCS) interface that you can use
when the MAC is configured for the TBI, RTBI, or SGMII PHY
interface.

...

You can include the optional PCS module in the DWC_gmac by
selecting the TBI, RTBI, or SGMII interface in coreConsultant
during configuration.
--->8--

But we use RGMII for communication with the PHY.
And from the quote above I would conclude that for RGMII we should
have PCS excluded from GMAC.

I just sent email to GMAC developers here in Synopsys and
hopefully will get some clarifications from them soon.

Probably correct solution is as simple as:
--->8--
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index a276a32d57f2..f4b67dc7d530 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -803,13 +803,7 @@ static void stmmac_check_pcs_mode(struct stmmac_priv *priv)
int interface = priv->plat->interface;
 
if (priv->dma_cap.pcs) {
-   if ((interface == PHY_INTERFACE_MODE_RGMII) ||
-   (interface == PHY_INTERFACE_MODE_RGMII_ID) ||
-   (interface == PHY_INTERFACE_MODE_RGMII_RXID) ||
-   (interface == PHY_INTERFACE_MODE_RGMII_TXID)) {
-   netdev_dbg(priv->dev, "PCS RGMII support enabled\n");
-   priv->hw->pcs = STMMAC_PCS_RGMII;
-   } else if (interface == PHY_INTERFACE_MODE_SGMII) {
+   if (interface == PHY_INTERFACE_MODE_SGMII) {
netdev_dbg(priv->dev, "PCS SGMII support enabled\n");
priv->hw->pcs = STMMAC_PCS_SGMII;
}
--->8--

> Also I wonder if, other version of the stmmac worked on this platform
> before.

It did work and still works. The only problem is we're getting
a lot of noise now about bogus link status change. That's because
this info is now in pr_info() compared to being previously in pr_debug().

-Alexey
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH 2/2] dmaengine: Add DW AXI DMAC driver

[Andy Shevchenko]

On Wed, 2017-01-25 at 18:34 +0300, Eugeniy Paltsev wrote:
> This patch adds support for the DW AXI DMAC controller.
> 
> DW AXI DMAC is a part of upcoming development board from Synopsys.
> 
> In this driver implementation only DMA_MEMCPY and DMA_SG transfers
> are supported.
> 

Few more comments on top of not addressed/answered yet.

> +static inline void axi_chan_disable(struct axi_dma_chan *chan)
> +{
> + u32 val;
> +
> + val = axi_dma_ioread32(chan->chip, DMAC_CHEN);
> + val &= ~(BIT(chan->id) << DMAC_CHAN_EN_SHIFT);
> + val |=  (BIT(chan->id) << DMAC_CHAN_EN_WE_SHIFT);
> + axi_dma_iowrite32(chan->chip, DMAC_CHEN, val);
> +}
> +
> +static inline void axi_chan_enable(struct axi_dma_chan *chan)
> +{
> + u32 val;
> +
> + val = axi_dma_ioread32(chan->chip, DMAC_CHEN);

> + val |= (BIT(chan->id) << DMAC_CHAN_EN_SHIFT |
> + BIT(chan->id) << DMAC_CHAN_EN_WE_SHIFT);
> 

Redundant parens.

> + axi_dma_iowrite32(chan->chip, DMAC_CHEN, val);
> +}


> +static void axi_desc_put(struct axi_dma_desc *desc)
> +{
> + struct axi_dma_chan *chan = desc->chan;
> + struct dw_axi_dma *dw = chan->chip->dw;
> + struct axi_dma_desc *child, *_next;
> + unsigned int descs_put = 0;
> +

> + if (unlikely(!desc))
> + return;

Would it be the case?

> +static void dma_chan_issue_pending(struct dma_chan *dchan)
> +{
> + struct axi_dma_chan *chan = dchan_to_axi_dma_chan(dchan);
> + unsigned long flags;
> +

> + dev_dbg(dchan2dev(dchan), "%s: %s\n", __func__,
> axi_chan_name(chan));

Messages like this kinda redundant.
Either you use function tracer and see them anyway, or you are using
Dynamic Debug, which may include function name.

Basically you wrote an equivalent to something like

dev_dbg(dev, "%s\n", channame);

> +
> + spin_lock_irqsave(>vc.lock, flags);
> + if (vchan_issue_pending(>vc))
> + axi_chan_start_first_queued(chan);
> + spin_unlock_irqrestore(>vc.lock, flags);

...and taking into account the function itself one might expect
something useful printed in _start_first_queued().

For some cases there is also dev_vdbg().

> +}
> +
> +static int dma_chan_alloc_chan_resources(struct dma_chan *dchan)
> +{
> + struct axi_dma_chan *chan = dchan_to_axi_dma_chan(dchan);
> +
> + /* ASSERT: channel is idle */
> + if (axi_chan_is_hw_enable(chan)) {
> + dev_err(chan2dev(chan), "%s is non-idle!\n",
> + axi_chan_name(chan));
> + return -EBUSY;
> + }
> +

> + dev_dbg(dchan2dev(dchan), "%s: allocating\n",
> axi_chan_name(chan));

Can you try to enable DMADEVICES_DEBUG and VDEBUG and see what is useful
and what is not?

Give a chance to function tracer as well.

> +static void dma_chan_free_chan_resources(struct dma_chan *dchan)
> +{
> + struct axi_dma_chan *chan = dchan_to_axi_dma_chan(dchan);
> +
> 

> + /* ASSERT: channel is idle */
> + if (axi_chan_is_hw_enable(chan))
> + dev_err(dchan2dev(dchan), "%s is non-idle!\n",
> + axi_chan_name(chan));

Yeah, as I said, dw_dmac is not a good example. And this one can be
managed by runtime PM I suppose.

> +
> +/* TODO: REVISIT: how we should choose AXI master for mem-to-mem
> transfer? */

Read datasheet for a SoC/platform? For dw_dmac is chosen with accordance
to hardware topology.

> + while (true) {

Usually it makes readability harder and rises a red flag to the code.

>   /* Manage transfer list (xfer_list) */
> + if (!first) {
> + first = desc;
> + } else {
> + list_add_tail(>xfer_list, 
> >xfer_list);
> + write_desc_llp(prev, desc->vd.tx.phys | lms);
> + }
> + prev = desc;
> +
> + /* update the lengths and addresses for the next loop
> cycle */
> + dst_len -= xfer_len;
> + src_len -= xfer_len;
> + dst_adr += xfer_len;
> + src_adr += xfer_len;
> +
> + total_len += xfer_len;

I would suggest to leave this on caller. At some point, if no one else
do this faster than me, I would like to introduce something like struct
dma_parms per DMA channel to allow caller prepare SG list suitable for
the DMA device.

> +
> +static struct dma_async_tx_descriptor *
> +dma_chan_prep_dma_memcpy(struct dma_chan *dchan, dma_addr_t dest,
> +  dma_addr_t src, size_t len, unsigned long
> flags)
> +{

Do you indeed have a use case for this except debugging and testing?

> + unsigned int nents = 1;
> + struct scatterlist dst_sg;
> + struct scatterlist src_sg;
> +
> + sg_init_table(_sg, nents);
> + sg_init_table(_sg, nents);
> +
> + sg_dma_address(_sg) = dest;
> + sg_dma_address(_sg) = src;
> +
> + sg_dma_len(_sg) = len;
> + sg_dma_len(_sg) = len;
> +
> + /* Implement memcpy transfer as sg transfer with single list
> */

> + return 

Re: [PATCH 2/2] dmaengine: Add DW AXI DMAC driver

[kbuild test robot]

Hi Eugeniy,

[auto build test ERROR on linus/master]
[also build test ERROR on v4.10-rc5 next-20170125]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Eugeniy-Paltsev/dmaengine-Add-DW-AXI-DMAC-driver/20170126-000653
config: i386-allmodconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

All error/warnings (new ones prefixed by >>):

   In file included from drivers/dma/axi_dma_platform.c:26:0:
>> include/linux/module.h:130:27: error: redefinition of '__inittest'
 static inline initcall_t __inittest(void)  \
  ^
>> include/linux/module.h:115:30: note: in expansion of macro 'module_init'
#define subsys_initcall(fn)  module_init(fn)
 ^~~
>> drivers/dma/axi_dma_platform.c:1050:1: note: in expansion of macro 
>> 'subsys_initcall'
subsys_initcall(dw_init);
^~~
   include/linux/module.h:130:27: note: previous definition of '__inittest' was 
here
 static inline initcall_t __inittest(void)  \
  ^
   include/linux/device.h:1463:1: note: in expansion of macro 'module_init'
module_init(__driver##_init); \
^~~
   include/linux/platform_device.h:228:2: note: in expansion of macro 
'module_driver'
 module_driver(__platform_driver, platform_driver_register, \
 ^
>> drivers/dma/axi_dma_platform.c:1044:1: note: in expansion of macro 
>> 'module_platform_driver'
module_platform_driver(dw_driver);
^~
>> include/linux/module.h:132:6: error: redefinition of 'init_module'
 int init_module(void) __attribute__((alias(#initfn)));
 ^
>> include/linux/module.h:115:30: note: in expansion of macro 'module_init'
#define subsys_initcall(fn)  module_init(fn)
 ^~~
>> drivers/dma/axi_dma_platform.c:1050:1: note: in expansion of macro 
>> 'subsys_initcall'
subsys_initcall(dw_init);
^~~
   include/linux/module.h:132:6: note: previous definition of 'init_module' was 
here
 int init_module(void) __attribute__((alias(#initfn)));
 ^
   include/linux/device.h:1463:1: note: in expansion of macro 'module_init'
module_init(__driver##_init); \
^~~
   include/linux/platform_device.h:228:2: note: in expansion of macro 
'module_driver'
 module_driver(__platform_driver, platform_driver_register, \
 ^
>> drivers/dma/axi_dma_platform.c:1044:1: note: in expansion of macro 
>> 'module_platform_driver'
module_platform_driver(dw_driver);
^~
>> include/linux/module.h:136:27: error: redefinition of '__exittest'
 static inline exitcall_t __exittest(void)  \
  ^
>> drivers/dma/axi_dma_platform.c:1056:1: note: in expansion of macro 
>> 'module_exit'
module_exit(dw_exit);
^~~
   include/linux/module.h:136:27: note: previous definition of '__exittest' was 
here
 static inline exitcall_t __exittest(void)  \
  ^
   include/linux/device.h:1468:1: note: in expansion of macro 'module_exit'
module_exit(__driver##_exit);
^~~
   include/linux/platform_device.h:228:2: note: in expansion of macro 
'module_driver'
 module_driver(__platform_driver, platform_driver_register, \
 ^
>> drivers/dma/axi_dma_platform.c:1044:1: note: in expansion of macro 
>> 'module_platform_driver'
module_platform_driver(dw_driver);
^~
>> include/linux/module.h:138:7: error: redefinition of 'cleanup_module'
 void cleanup_module(void) __attribute__((alias(#exitfn)));
  ^
>> drivers/dma/axi_dma_platform.c:1056:1: note: in expansion of macro 
>> 'module_exit'
module_exit(dw_exit);
^~~
   include/linux/module.h:138:7: note: previous definition of 'cleanup_module' 
was here
 void cleanup_module(void) __attribute__((alias(#exitfn)));
  ^
   include/linux/device.h:1468:1: note: in expansion of macro 'module_exit'
module_exit(__driver##_exit);
^~~
   include/linux/platform_device.h:228:2: note: in expansion of macro 
'module_driver'
 module_driver(__platform_driver, platform_driver_register, \
 ^
>> drivers/dma/axi_dma_platform.c:1044:1: note: in expansion of macro 
>> 'module_platform_driver'
module_platform_driver(dw_driver);
^~

vim +/__inittest +130 include/linux/module.h

0fd972a7 Paul Gortmaker 2015-05-01  109  #define early_initcall(fn) 
module_init(fn)
0fd972a7 Paul Gortmaker 2015-05-01  110  #define core_initcall(fn)  
module_init(fn)
0fd972a

Re: [PATCH 0/2] dmaengine: Add DW AXI DMAC driver

[Andy Shevchenko]

On Wed, 2017-01-25 at 18:34 +0300, Eugeniy Paltsev wrote:
> This patch series add support for the DW AXI DMAC controller.
> 
> DW AXI DMAC is a part of upcoming development board from Synopsys.
> 
> In this driver implementation only DMA_MEMCPY and DMA_SG transfers
> are supported.
> 
> Changes for v0:
>  * Switch to virt-dma API (according to previous RFC)
>  * Small fixies according to previous RFC

Yeah, seems you didn't address some of the comments and didn't comment
why...

>  * Add DT bindings
> 
> Eugeniy Paltsev (2):
>   dt-bindings: Document the Synopsys DW AXI DMA bindings
>   dmaengine: Add DW AXI DMAC driver
> 
>  .../devicetree/bindings/dma/snps,axi-dw-dmac.txt   |   33 +
>  drivers/dma/Kconfig|8 +
>  drivers/dma/Makefile   |1 +

>  drivers/dma/axi_dma_platform.c | 1060
> 

This surprises me. I would expect more then 100+ LOC reduction when
switched to virt-dma API. Can you double check that you are using it
effectively?

>  drivers/dma/axi_dma_platform.h |  124 +++
>  drivers/dma/axi_dma_platform_reg.h |  189 
>  6 files changed, 1415 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/dma/snps,axi-dw-
> dmac.txt
>  create mode 100644 drivers/dma/axi_dma_platform.c
>  create mode 100644 drivers/dma/axi_dma_platform.h
>  create mode 100644 drivers/dma/axi_dma_platform_reg.h
> 

-- 
Andy Shevchenko 
Intel Finland Oy

___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

[PATCH 0/2] dmaengine: Add DW AXI DMAC driver

[Eugeniy Paltsev]

This patch series add support for the DW AXI DMAC controller.

DW AXI DMAC is a part of upcoming development board from Synopsys.

In this driver implementation only DMA_MEMCPY and DMA_SG transfers
are supported.

Changes for v0:
 * Switch to virt-dma API (according to previous RFC)
 * Small fixies according to previous RFC
 * Add DT bindings

Eugeniy Paltsev (2):
  dt-bindings: Document the Synopsys DW AXI DMA bindings
  dmaengine: Add DW AXI DMAC driver

 .../devicetree/bindings/dma/snps,axi-dw-dmac.txt   |   33 +
 drivers/dma/Kconfig|8 +
 drivers/dma/Makefile   |1 +
 drivers/dma/axi_dma_platform.c | 1060 
 drivers/dma/axi_dma_platform.h |  124 +++
 drivers/dma/axi_dma_platform_reg.h |  189 
 6 files changed, 1415 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/dma/snps,axi-dw-dmac.txt
 create mode 100644 drivers/dma/axi_dma_platform.c
 create mode 100644 drivers/dma/axi_dma_platform.h
 create mode 100644 drivers/dma/axi_dma_platform_reg.h

-- 
2.5.5


___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc