From: Giuseppe CAVALLARO <peppe.cavallaro@st.com>
To: Niklas Cassel <niklas.cassel@axis.com>,
Alexandre Torgue <alexandre.torgue@st.com>
Cc: <netdev@vger.kernel.org>
Subject: Re: stmmac DT property snps,axi_all
Date: Mon, 12 Dec 2016 15:18:47 +0100 [thread overview]
Message-ID: <0546490f-8f8c-a22e-db7b-eac521c7ff27@st.com> (raw)
In-Reply-To: <e0362693-4ae9-b3d6-3955-c72df7a1b0c0@axis.com>
Please Niklas
when you send the patch, add my
Acked-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
On 12/9/2016 10:53 AM, Niklas Cassel wrote:
> On 12/09/2016 10:20 AM, Niklas Cassel wrote:
>> On 12/08/2016 02:36 PM, Alexandre Torgue wrote:
>>> Hi Niklas,
>>>
>>> On 12/05/2016 05:18 PM, Niklas Cassel wrote:
>>>> Hello Giuseppe
>>>>
>>>>
>>>> I'm trying to figure out what snps,axi_all is supposed to represent.
>>>>
>>>> It appears that the value is saved, but never used in the code.
>>>>
>>>> Looking at the register specification, I'm guessing that it represents
>>>> Address-Aligned Beats, but there is already the property snps,aal
>>>> for that.
>>> IMO, it is not useful. Indeed AXI_AAL is a read only bit (in AXI bus mode register) and reflects the aal bit in DMA bus register.
>>> As you know we use "snps,aal" to set aal bit in DMA bus register.
>>> So "snps,axi_all" entry seems useless. Let's see with Peppe.
>> Ok, I see. GMAC and GMAC4 is different here.
>>
>> For GMAC4 AAL only exists in DMA_SYS_BUS_MODE.
>> It's not reflected anywhere else.
>>
>> The code is correct in the driver.
>>
>> If snps,axi_all is just created for a read-only register,
>> and it is currently never used in the code,
>> while we have snps,aal, which is correct and works,
>> I guess it should be ok to remove snps,axi_all.
>>
>> I can cook up a patch.
>>
>
> Here we go :)
>
> I will send it as a real patch once net-next reopens.
>
>
>>From defc01cb7c22611b89d9cf1fcae72544092bd62c Mon Sep 17 00:00:00 2001
> From: Niklas Cassel <niklas.cassel@axis.com>
> Date: Fri, 9 Dec 2016 10:27:00 +0100
> Subject: [PATCH net-next] net: stmmac: remove unused duplicate property
> snps,axi_all
>
> For core revision 3.x Address-Aligned Beats is available in two registers.
> The DT property snps,aal was created for AAL in the DMA bus register,
> which is a read/write bit.
> The DT property snps,axi_all was created for AXI_AAL in the AXI bus mode
> register, which is a read only bit that reflects the value of AAL in the
> DMA bus register.
>
> Since the value of snps,axi_all is never used in the driver,
> and since the property was created for a bit that is read only,
> it should be safe to remove the property.
>
> Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
> ---
> Documentation/devicetree/bindings/net/stmmac.txt | 1 -
> drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 1 -
> include/linux/stmmac.h | 1 -
> 3 files changed, 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/stmmac.txt b/Documentation/devicetree/bindings/net/stmmac.txt
> index 128da752fec9..c3d2fd480a1b 100644
> --- a/Documentation/devicetree/bindings/net/stmmac.txt
> +++ b/Documentation/devicetree/bindings/net/stmmac.txt
> @@ -65,7 +65,6 @@ Optional properties:
> - snps,wr_osr_lmt: max write outstanding req. limit
> - snps,rd_osr_lmt: max read outstanding req. limit
> - snps,kbbe: do not cross 1KiB boundary.
> - - snps,axi_all: align address
> - snps,blen: this is a vector of supported burst length.
> - snps,fb: fixed-burst
> - snps,mb: mixed-burst
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> index 082cd48db6a7..60ba8993c650 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> @@ -121,7 +121,6 @@ static struct stmmac_axi *stmmac_axi_setup(struct platform_device *pdev)
> axi->axi_lpi_en = of_property_read_bool(np, "snps,lpi_en");
> axi->axi_xit_frm = of_property_read_bool(np, "snps,xit_frm");
> axi->axi_kbbe = of_property_read_bool(np, "snps,axi_kbbe");
> - axi->axi_axi_all = of_property_read_bool(np, "snps,axi_all");
> axi->axi_fb = of_property_read_bool(np, "snps,axi_fb");
> axi->axi_mb = of_property_read_bool(np, "snps,axi_mb");
> axi->axi_rb = of_property_read_bool(np, "snps,axi_rb");
> diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
> index 266dab9ad782..889e0e9a3f1c 100644
> --- a/include/linux/stmmac.h
> +++ b/include/linux/stmmac.h
> @@ -103,7 +103,6 @@ struct stmmac_axi {
> u32 axi_wr_osr_lmt;
> u32 axi_rd_osr_lmt;
> bool axi_kbbe;
> - bool axi_axi_all;
> u32 axi_blen[AXI_BLEN];
> bool axi_fb;
> bool axi_mb;
>
next prev parent reply other threads:[~2016-12-12 14:18 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-12-05 16:18 stmmac DT property snps,axi_all Niklas Cassel
2016-12-08 13:36 ` Alexandre Torgue
2016-12-09 9:20 ` Niklas Cassel
2016-12-09 9:53 ` Niklas Cassel
2016-12-09 16:06 ` Alexandre Torgue
2016-12-12 13:16 ` Giuseppe CAVALLARO
2016-12-12 14:18 ` Giuseppe CAVALLARO [this message]
2016-12-13 6:47 ` Giuseppe CAVALLARO
2016-12-13 8:26 ` Niklas Cassel
2016-12-13 8:46 ` Giuseppe CAVALLARO
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=0546490f-8f8c-a22e-db7b-eac521c7ff27@st.com \
--to=peppe.cavallaro@st.com \
--cc=alexandre.torgue@st.com \
--cc=netdev@vger.kernel.org \
--cc=niklas.cassel@axis.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).