Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1538408
| From | Alexandre Torgue <alexandre.torgue@st.com> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [PATCH v3 3/6] net: stmmac: stmmac_platform: fix parsing of DT binding |
| Date | 2016-12-08 11:00 +0100 |
| Message-ID | <sM3EB-4x2-7@gated-at.bofh.it> (permalink) |
| References | <sLLol-1fC-1@gated-at.bofh.it> <sLLol-1fC-13@gated-at.bofh.it> <sM2Se-4hp-11@gated-at.bofh.it> <sM3uW-4u1-7@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
Hi
On 12/08/2016 10:46 AM, Niklas Cassel wrote:
> On 12/08/2016 10:02 AM, Alexandre Torgue wrote:
>> Hi Niklas
>>
>> On 12/07/2016 03:20 PM, Niklas Cassel wrote:
>>> From: Niklas Cassel <niklas.cassel@axis.com>
>>>
>>> commit 64c3b252e9fc ("net: stmmac: fixed the pbl setting with DT")
>>> changed the parsing of the DT binding.
>>>
>>> Before 64c3b252e9fc, snps,fixed-burst and snps,mixed-burst were parsed
>>> regardless if the property snps,pbl existed or not.
>>> After the commit, fixed burst and mixed burst are only parsed if
>>> snps,pbl exists. Now when snps,aal has been added, it too is only
>>> parsed if snps,pbl exists.
>>>
>>> Since the DT binding does not specify that fixed burst, mixed burst
>>> or aal depend on snps,pbl being specified, undo changes introduced
>>> by 64c3b252e9fc.
>>>
>>> The issue commit 64c3b252e9fc ("net: stmmac: fixed the pbl setting with
>>> DT") tries to address is solved in another way:
>>> The databook specifies that all values other than
>>> 1, 2, 4, 8, 16, or 32 results in undefined behavior,
>>> so snps,pbl = <0> is invalid.
>>>
>>> If pbl is 0 after parsing, set pbl to DEFAULT_DMA_PBL.
>>> This handles the case where the property is omitted, and also handles
>>> the case where the property is specified without any data.
>>>
>>> Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
>>> ---
>>> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 4 +--
>>> .../net/ethernet/stmicro/stmmac/stmmac_platform.c | 29 +++++++++++-----------
>>> 2 files changed, 17 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>> index b1e42ddf0370..b5188122bc15 100644
>>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>> @@ -1581,8 +1581,8 @@ static int stmmac_init_dma_engine(struct stmmac_priv *priv)
>>> int atds = 0;
>>> int ret = 0;
>>>
>>> - if (!priv->plat->dma_cfg) {
>>> - dev_err(priv->device, "DMA configuration not found\n");
>>> + if (!priv->plat->dma_cfg || !priv->plat->dma_cfg->pbl) {
>>
>> How "priv->plat->dma_cfg->pbl" could be equal to 0 if you force it to DEFAULT_DMA_PBL in "stmmac_probe_config_dt" in case of DT doesn't set pbl value?
>
> The PCI glue code does not call stmmac_probe_config_dt.
>
> Also any glue driver could override the value set by stmmac_probe_config_dt
> before calling stmmac_dvr_probe. So I guess if we want any trustworthy
> sanity-checking, it actually has to be done in stmmac_main.c.
Ok I see, it is more safe. You can add my Acked-by.
Thanks
Alex
>
>
>>
>>
>>> + dev_err(priv->device, "Invalid DMA configuration\n");
>>> return -EINVAL;
>>> }
>>>
>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>>> index d3b6f92f350a..81800f23a9c4 100644
>>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>>> @@ -304,21 +304,22 @@ stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
>>> plat->force_sf_dma_mode = 1;
>>> }
>>>
>>> - if (of_find_property(np, "snps,pbl", NULL)) {
>>> - dma_cfg = devm_kzalloc(&pdev->dev, sizeof(*dma_cfg),
>>> - GFP_KERNEL);
>>> - if (!dma_cfg) {
>>> - stmmac_remove_config_dt(pdev, plat);
>>> - return ERR_PTR(-ENOMEM);
>>> - }
>>> - plat->dma_cfg = dma_cfg;
>>> - of_property_read_u32(np, "snps,pbl", &dma_cfg->pbl);
>>> - dma_cfg->aal = of_property_read_bool(np, "snps,aal");
>>> - dma_cfg->fixed_burst =
>>> - of_property_read_bool(np, "snps,fixed-burst");
>>> - dma_cfg->mixed_burst =
>>> - of_property_read_bool(np, "snps,mixed-burst");
>>> + dma_cfg = devm_kzalloc(&pdev->dev, sizeof(*dma_cfg),
>>> + GFP_KERNEL);
>>> + if (!dma_cfg) {
>>> + stmmac_remove_config_dt(pdev, plat);
>>> + return ERR_PTR(-ENOMEM);
>>> }
>>> + plat->dma_cfg = dma_cfg;
>>> +
>>> + of_property_read_u32(np, "snps,pbl", &dma_cfg->pbl);
>>> + if (!dma_cfg->pbl)
>>> + dma_cfg->pbl = DEFAULT_DMA_PBL;
>>> +
>>> + dma_cfg->aal = of_property_read_bool(np, "snps,aal");
>>> + dma_cfg->fixed_burst = of_property_read_bool(np, "snps,fixed-burst");
>>> + dma_cfg->mixed_burst = of_property_read_bool(np, "snps,mixed-burst");
>>> +
>>> plat->force_thresh_dma_mode = of_property_read_bool(np, "snps,force_thresh_dma_mode");
>>> if (plat->force_thresh_dma_mode) {
>>> plat->force_sf_dma_mode = 0;
>>>
>
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
[PATCH v3 0/6] net: stmmac: make DMA programmable burst length more configurable Niklas Cassel <niklas.cassel@axis.com> - 2016-12-07 15:30 +0100
[PATCH v3 3/6] net: stmmac: stmmac_platform: fix parsing of DT binding Niklas Cassel <niklas.cassel@axis.com> - 2016-12-07 15:30 +0100
Re: [PATCH v3 3/6] net: stmmac: stmmac_platform: fix parsing of DT binding Alexandre Torgue <alexandre.torgue@st.com> - 2016-12-08 10:10 +0100
Re: [PATCH v3 3/6] net: stmmac: stmmac_platform: fix parsing of DT binding Niklas Cassel <niklas.cassel@axis.com> - 2016-12-08 10:50 +0100
Re: [PATCH v3 3/6] net: stmmac: stmmac_platform: fix parsing of DT binding Alexandre Torgue <alexandre.torgue@st.com> - 2016-12-08 11:00 +0100
[PATCH v3 6/6] net: smmac: allow configuring lower pbl values Niklas Cassel <niklas.cassel@axis.com> - 2016-12-07 15:30 +0100
Re: [PATCH v3 6/6] net: smmac: allow configuring lower pbl values Alexandre Torgue <alexandre.torgue@st.com> - 2016-12-08 11:50 +0100
Re: [PATCH v3 6/6] net: smmac: allow configuring lower pbl values Andreas Färber <afaerber@suse.de> - 2016-12-08 15:10 +0100
Re: [PATCH v3 6/6] net: smmac: allow configuring lower pbl values David Miller <davem@davemloft.net> - 2016-12-08 16:20 +0100
[PATCH v3 1/6] net: stmmac: return error if no DMA configuration is found Niklas Cassel <niklas.cassel@axis.com> - 2016-12-07 15:30 +0100
Re: [PATCH v3 1/6] net: stmmac: return error if no DMA configuration is found Alexandre Torgue <alexandre.torgue@st.com> - 2016-12-08 11:50 +0100
Re: [PATCH v3 1/6] net: stmmac: return error if no DMA configuration is found David Miller <davem@davemloft.net> - 2016-12-08 16:20 +0100
Re: [PATCH v3 1/6] net: stmmac: return error if no DMA configuration is found Alexandre Torgue <alexandre.torgue@st.com> - 2016-12-08 16:50 +0100
[PATCH v3 2/6] net: stmmac: simplify the common DMA init API Niklas Cassel <niklas.cassel@axis.com> - 2016-12-07 15:30 +0100
Re: [PATCH v3 2/6] net: stmmac: simplify the common DMA init API Alexandre Torgue <alexandre.torgue@st.com> - 2016-12-08 10:00 +0100
[PATCH v3 5/6] net: stmmac: add support for independent DMA pbl for tx/rx Niklas Cassel <niklas.cassel@axis.com> - 2016-12-07 15:30 +0100
Re: [PATCH v3 5/6] net: stmmac: add support for independent DMA pbl for tx/rx Alexandre Torgue <alexandre.torgue@st.com> - 2016-12-08 11:40 +0100
[PATCH v3 4/6] net: stmmac: dwmac1000: fix define DMA_BUS_MODE_RPBL_MASK Niklas Cassel <niklas.cassel@axis.com> - 2016-12-07 15:30 +0100
Re: [PATCH v3 4/6] net: stmmac: dwmac1000: fix define DMA_BUS_MODE_RPBL_MASK Alexandre Torgue <alexandre.torgue@st.com> - 2016-12-08 10:20 +0100
Re: [PATCH v3 0/6] net: stmmac: make DMA programmable burst length more configurable David Miller <davem@davemloft.net> - 2016-12-08 19:10 +0100
csiph-web