* [PATCH net-next v2 1/2] net: stmmac: Only enable enhanced addressing mode when needed
@ 2019-09-09 15:25 Thierry Reding
2019-09-09 15:25 ` [PATCH net-next v2 2/2] net: stmmac: Support enhanced addressing mode for DWMAC 4.10 Thierry Reding
2019-09-09 16:07 ` [PATCH net-next v2 1/2] net: stmmac: Only enable enhanced addressing mode when needed Jose Abreu
0 siblings, 2 replies; 12+ messages in thread
From: Thierry Reding @ 2019-09-09 15:25 UTC (permalink / raw)
To: David S . Miller
Cc: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, Jon Hunter,
Bitan Biswas, netdev, linux-tegra
From: Thierry Reding <treding@nvidia.com>
Enhanced addressing mode is only required when more than 32 bits need to
be addressed. Add a DMA configuration parameter to enable this mode only
when needed.
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c | 5 ++++-
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 6 ++++++
include/linux/stmmac.h | 1 +
3 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
index 64956465c030..3e00fd8befcf 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
@@ -27,7 +27,10 @@ static void dwxgmac2_dma_init(void __iomem *ioaddr,
if (dma_cfg->aal)
value |= XGMAC_AAL;
- writel(value | XGMAC_EAME, ioaddr + XGMAC_DMA_SYSBUS_MODE);
+ if (dma_cfg->eame)
+ value |= XGMAC_EAME;
+
+ writel(value, ioaddr + XGMAC_DMA_SYSBUS_MODE);
}
static void dwxgmac2_dma_init_chan(void __iomem *ioaddr,
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 06ccd216ae90..ecd461207dbc 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -4497,6 +4497,12 @@ int stmmac_dvr_probe(struct device *device,
if (!ret) {
dev_info(priv->device, "Using %d bits DMA width\n",
priv->dma_cap.addr64);
+
+ /*
+ * If more than 32 bits can be addressed, make sure to
+ * enable enhanced addressing mode.
+ */
+ priv->plat->dma_cfg->eame = true;
} else {
ret = dma_set_mask_and_coherent(device, DMA_BIT_MASK(32));
if (ret) {
diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index 7ad7ae35cf88..d300ac907c76 100644
--- a/include/linux/stmmac.h
+++ b/include/linux/stmmac.h
@@ -92,6 +92,7 @@ struct stmmac_dma_cfg {
int fixed_burst;
int mixed_burst;
bool aal;
+ bool eame;
};
#define AXI_BLEN 7
--
2.23.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH net-next v2 2/2] net: stmmac: Support enhanced addressing mode for DWMAC 4.10
2019-09-09 15:25 [PATCH net-next v2 1/2] net: stmmac: Only enable enhanced addressing mode when needed Thierry Reding
@ 2019-09-09 15:25 ` Thierry Reding
2019-09-09 16:05 ` Jose Abreu
2019-09-09 16:07 ` [PATCH net-next v2 1/2] net: stmmac: Only enable enhanced addressing mode when needed Jose Abreu
1 sibling, 1 reply; 12+ messages in thread
From: Thierry Reding @ 2019-09-09 15:25 UTC (permalink / raw)
To: David S . Miller
Cc: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, Jon Hunter,
Bitan Biswas, netdev, linux-tegra
From: Thierry Reding <treding@nvidia.com>
The address width of the controller can be read from hardware feature
registers much like on XGMAC. Add support for parsing the ADDR64 field
so that the DMA mask can be set accordingly.
This avoids getting swiotlb involved for DMA on Tegra186 and later.
Also make sure that the upper 32 bits of the DMA address are written to
the DMA descriptors when enhanced addressing mode is used. Similarily,
for each channel, the upper 32 bits of the DMA descriptor ring's base
address also need to be programmed to make sure the correct memory can
be fetched when the DMA descriptor ring is located beyond the 32-bit
boundary.
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
Changes in v2:
- also program the upper 32 bits of the DMA descriptor base address for
each channel
drivers/net/ethernet/stmicro/stmmac/dwmac4.h | 1 +
.../ethernet/stmicro/stmmac/dwmac4_descs.c | 4 +--
.../net/ethernet/stmicro/stmmac/dwmac4_dma.c | 28 +++++++++++++++++++
.../net/ethernet/stmicro/stmmac/dwmac4_dma.h | 3 ++
4 files changed, 34 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
index 2ed11a581d80..f634fa09dffc 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
@@ -183,6 +183,7 @@ enum power_event {
#define GMAC_HW_HASH_TB_SZ GENMASK(25, 24)
#define GMAC_HW_FEAT_AVSEL BIT(20)
#define GMAC_HW_TSOEN BIT(18)
+#define GMAC_HW_ADDR64 GENMASK(15, 14)
#define GMAC_HW_TXFIFOSIZE GENMASK(10, 6)
#define GMAC_HW_RXFIFOSIZE GENMASK(4, 0)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
index dbde23e7e169..d546041d2fcd 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
@@ -431,8 +431,8 @@ static void dwmac4_get_addr(struct dma_desc *p, unsigned int *addr)
static void dwmac4_set_addr(struct dma_desc *p, dma_addr_t addr)
{
- p->des0 = cpu_to_le32(addr);
- p->des1 = 0;
+ p->des0 = cpu_to_le32(lower_32_bits(addr));
+ p->des1 = cpu_to_le32(upper_32_bits(addr));
}
static void dwmac4_clear(struct dma_desc *p)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
index 3ed5508586ef..e77410f21d7d 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
@@ -79,6 +79,10 @@ static void dwmac4_dma_init_rx_chan(void __iomem *ioaddr,
value = value | (rxpbl << DMA_BUS_MODE_RPBL_SHIFT);
writel(value, ioaddr + DMA_CHAN_RX_CONTROL(chan));
+ if (dma_cfg->eame)
+ writel(upper_32_bits(dma_rx_phy),
+ ioaddr + DMA_CHAN_RX_BASE_ADDR_HI(chan));
+
writel(lower_32_bits(dma_rx_phy), ioaddr + DMA_CHAN_RX_BASE_ADDR(chan));
}
@@ -97,6 +101,10 @@ static void dwmac4_dma_init_tx_chan(void __iomem *ioaddr,
writel(value, ioaddr + DMA_CHAN_TX_CONTROL(chan));
+ if (dma_cfg->eame)
+ writel(upper_32_bits(dma_tx_phy),
+ ioaddr + DMA_CHAN_TX_BASE_ADDR_HI(chan));
+
writel(lower_32_bits(dma_tx_phy), ioaddr + DMA_CHAN_TX_BASE_ADDR(chan));
}
@@ -132,6 +140,9 @@ static void dwmac4_dma_init(void __iomem *ioaddr,
if (dma_cfg->aal)
value |= DMA_SYS_BUS_AAL;
+ if (dma_cfg->eame)
+ value |= DMA_SYS_BUS_EAME;
+
writel(value, ioaddr + DMA_SYS_BUS_MODE);
}
@@ -354,6 +365,23 @@ static void dwmac4_get_hw_feature(void __iomem *ioaddr,
dma_cap->hash_tb_sz = (hw_cap & GMAC_HW_HASH_TB_SZ) >> 24;
dma_cap->av = (hw_cap & GMAC_HW_FEAT_AVSEL) >> 20;
dma_cap->tsoen = (hw_cap & GMAC_HW_TSOEN) >> 18;
+
+ dma_cap->addr64 = (hw_cap & GMAC_HW_ADDR64) >> 14;
+ switch (dma_cap->addr64) {
+ case 0:
+ dma_cap->addr64 = 32;
+ break;
+ case 1:
+ dma_cap->addr64 = 40;
+ break;
+ case 2:
+ dma_cap->addr64 = 48;
+ break;
+ default:
+ dma_cap->addr64 = 32;
+ break;
+ }
+
/* RX and TX FIFO sizes are encoded as log2(n / 128). Undo that by
* shifting and store the sizes in bytes.
*/
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.h b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.h
index b66da0237d2a..5299fa1001a3 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.h
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.h
@@ -65,6 +65,7 @@
#define DMA_SYS_BUS_MB BIT(14)
#define DMA_AXI_1KBBE BIT(13)
#define DMA_SYS_BUS_AAL BIT(12)
+#define DMA_SYS_BUS_EAME BIT(11)
#define DMA_AXI_BLEN256 BIT(7)
#define DMA_AXI_BLEN128 BIT(6)
#define DMA_AXI_BLEN64 BIT(5)
@@ -91,7 +92,9 @@
#define DMA_CHAN_CONTROL(x) DMA_CHANX_BASE_ADDR(x)
#define DMA_CHAN_TX_CONTROL(x) (DMA_CHANX_BASE_ADDR(x) + 0x4)
#define DMA_CHAN_RX_CONTROL(x) (DMA_CHANX_BASE_ADDR(x) + 0x8)
+#define DMA_CHAN_TX_BASE_ADDR_HI(x) (DMA_CHANX_BASE_ADDR(x) + 0x10)
#define DMA_CHAN_TX_BASE_ADDR(x) (DMA_CHANX_BASE_ADDR(x) + 0x14)
+#define DMA_CHAN_RX_BASE_ADDR_HI(x) (DMA_CHANX_BASE_ADDR(x) + 0x18)
#define DMA_CHAN_RX_BASE_ADDR(x) (DMA_CHANX_BASE_ADDR(x) + 0x1c)
#define DMA_CHAN_TX_END_ADDR(x) (DMA_CHANX_BASE_ADDR(x) + 0x20)
#define DMA_CHAN_RX_END_ADDR(x) (DMA_CHANX_BASE_ADDR(x) + 0x28)
--
2.23.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* RE: [PATCH net-next v2 2/2] net: stmmac: Support enhanced addressing mode for DWMAC 4.10
2019-09-09 15:25 ` [PATCH net-next v2 2/2] net: stmmac: Support enhanced addressing mode for DWMAC 4.10 Thierry Reding
@ 2019-09-09 16:05 ` Jose Abreu
2019-09-09 19:13 ` Thierry Reding
0 siblings, 1 reply; 12+ messages in thread
From: Jose Abreu @ 2019-09-09 16:05 UTC (permalink / raw)
To: Thierry Reding, David S . Miller
Cc: Giuseppe Cavallaro, Alexandre Torgue, Jon Hunter, Bitan Biswas,
netdev, linux-tegra
From: Thierry Reding <thierry.reding@gmail.com>
Date: Sep/09/2019, 16:25:46 (UTC+00:00)
> @@ -79,6 +79,10 @@ static void dwmac4_dma_init_rx_chan(void __iomem *ioaddr,
> value = value | (rxpbl << DMA_BUS_MODE_RPBL_SHIFT);
> writel(value, ioaddr + DMA_CHAN_RX_CONTROL(chan));
>
> + if (dma_cfg->eame)
There is no need for this check. If EAME is not enabled then upper 32
bits will be zero.
> + writel(upper_32_bits(dma_rx_phy),
> + ioaddr + DMA_CHAN_RX_BASE_ADDR_HI(chan));
> +
> writel(lower_32_bits(dma_rx_phy), ioaddr + DMA_CHAN_RX_BASE_ADDR(chan));
> }
> @@ -97,6 +101,10 @@ static void dwmac4_dma_init_tx_chan(void __iomem *ioaddr,
>
> writel(value, ioaddr + DMA_CHAN_TX_CONTROL(chan));
>
> + if (dma_cfg->eame)
Same here.
> + writel(upper_32_bits(dma_tx_phy),
> + ioaddr + DMA_CHAN_TX_BASE_ADDR_HI(chan));
> +
> writel(lower_32_bits(dma_tx_phy), ioaddr + DMA_CHAN_TX_BASE_ADDR(chan));
> }
Also, please provide a cover letter in next submission.
---
Thanks,
Jose Miguel Abreu
^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH net-next v2 1/2] net: stmmac: Only enable enhanced addressing mode when needed
2019-09-09 15:25 [PATCH net-next v2 1/2] net: stmmac: Only enable enhanced addressing mode when needed Thierry Reding
2019-09-09 15:25 ` [PATCH net-next v2 2/2] net: stmmac: Support enhanced addressing mode for DWMAC 4.10 Thierry Reding
@ 2019-09-09 16:07 ` Jose Abreu
2019-09-09 19:11 ` Thierry Reding
1 sibling, 1 reply; 12+ messages in thread
From: Jose Abreu @ 2019-09-09 16:07 UTC (permalink / raw)
To: Thierry Reding, David S . Miller
Cc: Giuseppe Cavallaro, Alexandre Torgue, Jon Hunter, Bitan Biswas,
netdev, linux-tegra
From: Thierry Reding <thierry.reding@gmail.com>
Date: Sep/09/2019, 16:25:45 (UTC+00:00)
> @@ -92,6 +92,7 @@ struct stmmac_dma_cfg {
> int fixed_burst;
> int mixed_burst;
> bool aal;
> + bool eame;
bools should not be used in struct's, please change to int.
---
Thanks,
Jose Miguel Abreu
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next v2 1/2] net: stmmac: Only enable enhanced addressing mode when needed
2019-09-09 16:07 ` [PATCH net-next v2 1/2] net: stmmac: Only enable enhanced addressing mode when needed Jose Abreu
@ 2019-09-09 19:11 ` Thierry Reding
2019-09-10 8:32 ` Jose Abreu
0 siblings, 1 reply; 12+ messages in thread
From: Thierry Reding @ 2019-09-09 19:11 UTC (permalink / raw)
To: Jose Abreu
Cc: David S . Miller, Giuseppe Cavallaro, Alexandre Torgue,
Jon Hunter, Bitan Biswas, netdev, linux-tegra
[-- Attachment #1: Type: text/plain, Size: 502 bytes --]
On Mon, Sep 09, 2019 at 04:07:04PM +0000, Jose Abreu wrote:
> From: Thierry Reding <thierry.reding@gmail.com>
> Date: Sep/09/2019, 16:25:45 (UTC+00:00)
>
> > @@ -92,6 +92,7 @@ struct stmmac_dma_cfg {
> > int fixed_burst;
> > int mixed_burst;
> > bool aal;
> > + bool eame;
>
> bools should not be used in struct's, please change to int.
Huh? Since when? "aal" right above it is also bool. Can you provide a
specific rationale for why we shouldn't use bool in structs?
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next v2 2/2] net: stmmac: Support enhanced addressing mode for DWMAC 4.10
2019-09-09 16:05 ` Jose Abreu
@ 2019-09-09 19:13 ` Thierry Reding
2019-09-10 8:35 ` Jose Abreu
0 siblings, 1 reply; 12+ messages in thread
From: Thierry Reding @ 2019-09-09 19:13 UTC (permalink / raw)
To: Jose Abreu
Cc: David S . Miller, Giuseppe Cavallaro, Alexandre Torgue,
Jon Hunter, Bitan Biswas, netdev, linux-tegra
[-- Attachment #1: Type: text/plain, Size: 1400 bytes --]
On Mon, Sep 09, 2019 at 04:05:52PM +0000, Jose Abreu wrote:
> From: Thierry Reding <thierry.reding@gmail.com>
> Date: Sep/09/2019, 16:25:46 (UTC+00:00)
>
> > @@ -79,6 +79,10 @@ static void dwmac4_dma_init_rx_chan(void __iomem *ioaddr,
> > value = value | (rxpbl << DMA_BUS_MODE_RPBL_SHIFT);
> > writel(value, ioaddr + DMA_CHAN_RX_CONTROL(chan));
> >
> > + if (dma_cfg->eame)
>
> There is no need for this check. If EAME is not enabled then upper 32
> bits will be zero.
The idea here was to potentially guard against this register not being
available on some revisions. Having the check here would avoid access to
the register if the device doesn't support enhanced addressing.
>
> > + writel(upper_32_bits(dma_rx_phy),
> > + ioaddr + DMA_CHAN_RX_BASE_ADDR_HI(chan));
> > +
> > writel(lower_32_bits(dma_rx_phy), ioaddr + DMA_CHAN_RX_BASE_ADDR(chan));
> > }
>
> > @@ -97,6 +101,10 @@ static void dwmac4_dma_init_tx_chan(void __iomem *ioaddr,
> >
> > writel(value, ioaddr + DMA_CHAN_TX_CONTROL(chan));
> >
> > + if (dma_cfg->eame)
>
> Same here.
>
> > + writel(upper_32_bits(dma_tx_phy),
> > + ioaddr + DMA_CHAN_TX_BASE_ADDR_HI(chan));
> > +
> > writel(lower_32_bits(dma_tx_phy), ioaddr + DMA_CHAN_TX_BASE_ADDR(chan));
> > }
>
> Also, please provide a cover letter in next submission.
Alright, will do.
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH net-next v2 1/2] net: stmmac: Only enable enhanced addressing mode when needed
2019-09-09 19:11 ` Thierry Reding
@ 2019-09-10 8:32 ` Jose Abreu
2019-09-10 13:54 ` Thierry Reding
0 siblings, 1 reply; 12+ messages in thread
From: Jose Abreu @ 2019-09-10 8:32 UTC (permalink / raw)
To: Thierry Reding, Jose Abreu
Cc: David S . Miller, Giuseppe Cavallaro, Alexandre Torgue,
Jon Hunter, Bitan Biswas, netdev, linux-tegra
From: Thierry Reding <thierry.reding@gmail.com>
Date: Sep/09/2019, 20:11:27 (UTC+00:00)
> On Mon, Sep 09, 2019 at 04:07:04PM +0000, Jose Abreu wrote:
> > From: Thierry Reding <thierry.reding@gmail.com>
> > Date: Sep/09/2019, 16:25:45 (UTC+00:00)
> >
> > > @@ -92,6 +92,7 @@ struct stmmac_dma_cfg {
> > > int fixed_burst;
> > > int mixed_burst;
> > > bool aal;
> > > + bool eame;
> >
> > bools should not be used in struct's, please change to int.
>
> Huh? Since when? "aal" right above it is also bool. Can you provide a
> specific rationale for why we shouldn't use bool in structs?
Please see https://lkml.org/lkml/2017/11/21/384.
---
Thanks,
Jose
Miguel Abreu
^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH net-next v2 2/2] net: stmmac: Support enhanced addressing mode for DWMAC 4.10
2019-09-09 19:13 ` Thierry Reding
@ 2019-09-10 8:35 ` Jose Abreu
2019-09-10 19:01 ` Florian Fainelli
0 siblings, 1 reply; 12+ messages in thread
From: Jose Abreu @ 2019-09-10 8:35 UTC (permalink / raw)
To: Thierry Reding, Jose Abreu
Cc: David S . Miller, Giuseppe Cavallaro, Alexandre Torgue,
Jon Hunter, Bitan Biswas, netdev, linux-tegra
From: Thierry Reding <thierry.reding@gmail.com>
Date: Sep/09/2019, 20:13:29 (UTC+00:00)
> On Mon, Sep 09, 2019 at 04:05:52PM +0000, Jose Abreu wrote:
> > From: Thierry Reding <thierry.reding@gmail.com>
> > Date: Sep/09/2019, 16:25:46 (UTC+00:00)
> >
> > > @@ -79,6 +79,10 @@ static void dwmac4_dma_init_rx_chan(void __iomem *ioaddr,
> > > value = value | (rxpbl << DMA_BUS_MODE_RPBL_SHIFT);
> > > writel(value, ioaddr + DMA_CHAN_RX_CONTROL(chan));
> > >
> > > + if (dma_cfg->eame)
> >
> > There is no need for this check. If EAME is not enabled then upper 32
> > bits will be zero.
>
> The idea here was to potentially guard against this register not being
> available on some revisions. Having the check here would avoid access to
> the register if the device doesn't support enhanced addressing.
I see your point but I don't think there will be any problems unless you
have some strange system that doesn't handle the write accesses to
unimplemented features properly ...
---
Thanks,
Jose Miguel Abreu
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next v2 1/2] net: stmmac: Only enable enhanced addressing mode when needed
2019-09-10 8:32 ` Jose Abreu
@ 2019-09-10 13:54 ` Thierry Reding
2019-09-10 17:25 ` Jose Abreu
0 siblings, 1 reply; 12+ messages in thread
From: Thierry Reding @ 2019-09-10 13:54 UTC (permalink / raw)
To: Jose Abreu
Cc: David S . Miller, Giuseppe Cavallaro, Alexandre Torgue,
Jon Hunter, Bitan Biswas, netdev, linux-tegra
[-- Attachment #1: Type: text/plain, Size: 1229 bytes --]
On Tue, Sep 10, 2019 at 08:32:38AM +0000, Jose Abreu wrote:
> From: Thierry Reding <thierry.reding@gmail.com>
> Date: Sep/09/2019, 20:11:27 (UTC+00:00)
>
> > On Mon, Sep 09, 2019 at 04:07:04PM +0000, Jose Abreu wrote:
> > > From: Thierry Reding <thierry.reding@gmail.com>
> > > Date: Sep/09/2019, 16:25:45 (UTC+00:00)
> > >
> > > > @@ -92,6 +92,7 @@ struct stmmac_dma_cfg {
> > > > int fixed_burst;
> > > > int mixed_burst;
> > > > bool aal;
> > > > + bool eame;
> > >
> > > bools should not be used in struct's, please change to int.
> >
> > Huh? Since when? "aal" right above it is also bool. Can you provide a
> > specific rationale for why we shouldn't use bool in structs?
>
> Please see https://lkml.org/lkml/2017/11/21/384.
The context is slightly different here. stmmac_dma_cfg exists once for
each of these ethernet devices in the system, and I would assume that in
the vast majority of cases there's exactly one such device in the system
so the potential size increase is very small. On the other hand, there
are potentially very many struct sched_dl_entity, so the size impact is
multiplied.
Anyway, if you insist I'll rewrite this to use an unsigned int bitfield.
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH net-next v2 1/2] net: stmmac: Only enable enhanced addressing mode when needed
2019-09-10 13:54 ` Thierry Reding
@ 2019-09-10 17:25 ` Jose Abreu
0 siblings, 0 replies; 12+ messages in thread
From: Jose Abreu @ 2019-09-10 17:25 UTC (permalink / raw)
To: Thierry Reding, Jose Abreu
Cc: David S . Miller, Giuseppe Cavallaro, Alexandre Torgue,
Jon Hunter, Bitan Biswas, netdev, linux-tegra
From: Thierry Reding <thierry.reding@gmail.com>
Date: Sep/10/2019, 14:54:27 (UTC+00:00)
> On Tue, Sep 10, 2019 at 08:32:38AM +0000, Jose Abreu wrote:
> > From: Thierry Reding <thierry.reding@gmail.com>
> > Date: Sep/09/2019, 20:11:27 (UTC+00:00)
> >
> > > On Mon, Sep 09, 2019 at 04:07:04PM +0000, Jose Abreu wrote:
> > > > From: Thierry Reding <thierry.reding@gmail.com>
> > > > Date: Sep/09/2019, 16:25:45 (UTC+00:00)
> > > >
> > > > > @@ -92,6 +92,7 @@ struct stmmac_dma_cfg {
> > > > > int fixed_burst;
> > > > > int mixed_burst;
> > > > > bool aal;
> > > > > + bool eame;
> > > >
> > > > bools should not be used in struct's, please change to int.
> > >
> > > Huh? Since when? "aal" right above it is also bool. Can you provide a
> > > specific rationale for why we shouldn't use bool in structs?
> >
> > Please see https://lkml.org/lkml/2017/11/21/384.
>
> The context is slightly different here. stmmac_dma_cfg exists once for
> each of these ethernet devices in the system, and I would assume that in
> the vast majority of cases there's exactly one such device in the system
> so the potential size increase is very small. On the other hand, there
> are potentially very many struct sched_dl_entity, so the size impact is
> multiplied.
>
> Anyway, if you insist I'll rewrite this to use an unsigned int bitfield.
For new code I would rather prefer "int" but I guess it's up to David to
decide this. I'm okay with both options as the check for this usage was
removed in checkpatch:
https://lkml.org/lkml/2019/1/10/975
---
Thanks,
Jose Miguel
Abreu
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next v2 2/2] net: stmmac: Support enhanced addressing mode for DWMAC 4.10
2019-09-10 8:35 ` Jose Abreu
@ 2019-09-10 19:01 ` Florian Fainelli
2019-09-11 9:15 ` Jose Abreu
0 siblings, 1 reply; 12+ messages in thread
From: Florian Fainelli @ 2019-09-10 19:01 UTC (permalink / raw)
To: Jose Abreu, Thierry Reding
Cc: David S . Miller, Giuseppe Cavallaro, Alexandre Torgue,
Jon Hunter, Bitan Biswas, netdev, linux-tegra
On 9/10/19 1:35 AM, Jose Abreu wrote:
> From: Thierry Reding <thierry.reding@gmail.com>
> Date: Sep/09/2019, 20:13:29 (UTC+00:00)
>
>> On Mon, Sep 09, 2019 at 04:05:52PM +0000, Jose Abreu wrote:
>>> From: Thierry Reding <thierry.reding@gmail.com>
>>> Date: Sep/09/2019, 16:25:46 (UTC+00:00)
>>>
>>>> @@ -79,6 +79,10 @@ static void dwmac4_dma_init_rx_chan(void __iomem *ioaddr,
>>>> value = value | (rxpbl << DMA_BUS_MODE_RPBL_SHIFT);
>>>> writel(value, ioaddr + DMA_CHAN_RX_CONTROL(chan));
>>>>
>>>> + if (dma_cfg->eame)
>>>
>>> There is no need for this check. If EAME is not enabled then upper 32
>>> bits will be zero.
>>
>> The idea here was to potentially guard against this register not being
>> available on some revisions. Having the check here would avoid access to
>> the register if the device doesn't support enhanced addressing.
>
> I see your point but I don't think there will be any problems unless you
> have some strange system that doesn't handle the write accesses to
> unimplemented features properly ...
Is not it then just safer to not do the write to a register that you do
not know how the implementation is going to respond to with one of a
target abort, timeout, decoding error, just dead lock?
Also, would it make sense to consider adding an #ifdef
CONFIG_PHYS_ADDR_T_64BIT plus the conditional check so that you can be
slightly more optimal in the hot-path here?
--
Florian
^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH net-next v2 2/2] net: stmmac: Support enhanced addressing mode for DWMAC 4.10
2019-09-10 19:01 ` Florian Fainelli
@ 2019-09-11 9:15 ` Jose Abreu
0 siblings, 0 replies; 12+ messages in thread
From: Jose Abreu @ 2019-09-11 9:15 UTC (permalink / raw)
To: Florian Fainelli, Jose Abreu, Thierry Reding
Cc: David S . Miller, Giuseppe Cavallaro, Alexandre Torgue,
Jon Hunter, Bitan Biswas, netdev, linux-tegra
From: Florian Fainelli <f.fainelli@gmail.com>
Date: Sep/10/2019, 20:01:01 (UTC+00:00)
> On 9/10/19 1:35 AM, Jose Abreu wrote:
> > From: Thierry Reding <thierry.reding@gmail.com>
> > Date: Sep/09/2019, 20:13:29 (UTC+00:00)
> >
> >> On Mon, Sep 09, 2019 at 04:05:52PM +0000, Jose Abreu wrote:
> >>> From: Thierry Reding <thierry.reding@gmail.com>
> >>> Date: Sep/09/2019, 16:25:46 (UTC+00:00)
> >>>
> >>>> @@ -79,6 +79,10 @@ static void dwmac4_dma_init_rx_chan(void __iomem *ioaddr,
> >>>> value = value | (rxpbl << DMA_BUS_MODE_RPBL_SHIFT);
> >>>> writel(value, ioaddr + DMA_CHAN_RX_CONTROL(chan));
> >>>>
> >>>> + if (dma_cfg->eame)
> >>>
> >>> There is no need for this check. If EAME is not enabled then upper 32
> >>> bits will be zero.
> >>
> >> The idea here was to potentially guard against this register not being
> >> available on some revisions. Having the check here would avoid access to
> >> the register if the device doesn't support enhanced addressing.
> >
> > I see your point but I don't think there will be any problems unless you
> > have some strange system that doesn't handle the write accesses to
> > unimplemented features properly ...
>
> Is not it then just safer to not do the write to a register that you do
> not know how the implementation is going to respond to with one of a
> target abort, timeout, decoding error, just dead lock?
I don't think any of these will ever happen. Notice that this is already
been done for a long time in some registers that may not exist in some
random HW config and there is also the point that this is a write
operation so Slave Error would only get triggered if we did a read.
> Also, would it make sense to consider adding an #ifdef
> CONFIG_PHYS_ADDR_T_64BIT plus the conditional check so that you can be
> slightly more optimal in the hot-path here?
Well, this is not hot-path. It's only done in HW open sequence. The
hot-path would be set_{rx/tx}_tail_ptr() but that's 32 bits only.
---
Thanks,
Jose Miguel Abreu
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2019-09-11 9:15 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-09 15:25 [PATCH net-next v2 1/2] net: stmmac: Only enable enhanced addressing mode when needed Thierry Reding
2019-09-09 15:25 ` [PATCH net-next v2 2/2] net: stmmac: Support enhanced addressing mode for DWMAC 4.10 Thierry Reding
2019-09-09 16:05 ` Jose Abreu
2019-09-09 19:13 ` Thierry Reding
2019-09-10 8:35 ` Jose Abreu
2019-09-10 19:01 ` Florian Fainelli
2019-09-11 9:15 ` Jose Abreu
2019-09-09 16:07 ` [PATCH net-next v2 1/2] net: stmmac: Only enable enhanced addressing mode when needed Jose Abreu
2019-09-09 19:11 ` Thierry Reding
2019-09-10 8:32 ` Jose Abreu
2019-09-10 13:54 ` Thierry Reding
2019-09-10 17:25 ` Jose Abreu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.