netdev - Re: stmmac DT property snps,axi

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 13 Dec 2016 07:47:48 +0100
From:   Giuseppe CAVALLARO <peppe.cavallaro@...com>
To:     Niklas Cassel <niklas.cassel@...s.com>,
        Alexandre Torgue <alexandre.torgue@...com>
CC:     <netdev@...r.kernel.org>
Subject: Re: stmmac DT property snps,axi_all

Hello Niklas, Alex,

my fault and a step behind... Current code is OK
when manage the AAL that, although it is passed from
the axi structure, it is always used to program,
for all the chip versions, the writable bit inside the
DMA_BUS_MODE register.
So I guess no extra patch is needed.

Regards
Peppe

On 12/12/2016 3:18 PM, Giuseppe CAVALLARO wrote:
> Please Niklas
>
> when you send the patch, add my
>
> Acked-by: Giuseppe Cavallaro <peppe.cavallaro@...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@...s.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@...s.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;
>>
>
>