netdev - Re: [PATCH 2/6] stmmac: Define MDC clock selection macros.

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]

Message-ID: <4F570BF4.5070706@st.com>
Date:	Wed, 07 Mar 2012 08:19:16 +0100
From:	Giuseppe CAVALLARO <peppe.cavallaro@...com>
To:	Deepak SIKRI <deepak.sikri@...com>
Cc:	spear-devel <spear-devel@...t.st.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH 2/6] stmmac: Define MDC clock selection macros.

On 3/7/2012 7:55 AM, Deepak SIKRI wrote:
> Hello Peppe,
> 
>>
>> I have some concerns about this patch.
>>
>> We want to have some defines to help on setting the clk_csr (that is is
>> a clk divisor).
>>
>> When you program the "CSR Clock Range" bits in the GMII Address Register
>> you can also set the bit 5 (not supported in older devices e.g. 3.41a).
>> In this case, the defines below do not cover all the cases, I mean:
>>
>> 1000 clk_csr_i/4
>> 1001 clk_csr_i/6
>> 1010 clk_csr_i/8
>> 1011 clk_csr_i/10
>> 1100 clk_csr_i/12
>> 1101 clk_csr_i/14
>> 1110 clk_csr_i/16
>> 1111 clk_csr_i/18
> 
> I agree that these macros have been missed. But lets take the change
> suggested as a separate patch,
> as this would then be integrated along with the driver.
> The driver by default is considering the 2.5MHz MDIO clock option only.
> In this case we require an extra
> variable to differentiate specification which is higher than IEEE spec
> of 2.5MHz.

ok, we will have them in a another patch. At any rate, I ask you to add
a FIXME in the code and detail this issue in the patch comment.

> 
>>> +/* MDC Clock Selection define*/
>>> +#define    STMMAC_CLK_RANGE_60_100M    0    /* MDC = Clk/42 */
>>> +#define    STMMAC_CLK_RANGE_100_150M    1    /* MDC = Clk/62 */
>>> +#define    STMMAC_CLK_RANGE_20_35M        2    /* MDC = Clk/16 */
>>> +#define    STMMAC_CLK_RANGE_35_60M        3    /* MDC = Clk/26 */
>>> +#define    STMMAC_CLK_RANGE_150_250M    4    /* MDC = Clk/102 */
>>> +#define    STMMAC_CLK_RANGE_250_300M    5    /* MDC = Clk/122 */
>> I suggest you to rename these macros as:
>>
>> #define STMMAC_CSR_60_100M    0    /* MDC = Clk/42 */
>> ...
> 
> Ok
> 
>>
>> Also, macros CSR_F_20M should be totally removed.
> 
> ok
> 
>> Peppe
>>
>>> +
>>>   /* Platfrom data for platform device structure's platform_data
>>> field */
>>>
>>>   struct stmmac_mdio_bus_data {
> Regards
> Deepak
> 

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html