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  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:   Sat, 23 May 2020 16:40:22 -0500
From:   Dan Murphy <dmurphy@...com>
To:     Andrew Lunn <andrew@...n.ch>
CC:     Florian Fainelli <f.fainelli@...il.com>, <hkallweit1@...il.com>,
        <davem@...emloft.net>, <robh@...nel.org>, <netdev@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <devicetree@...r.kernel.org>
Subject: Re: [PATCH net-next v2 4/4] net: dp83869: Add RGMII internal delay
 configuration

Andrew

On 5/23/20 10:09 AM, Andrew Lunn wrote:
>>>> +	dp83869->tx_id_delay = DP83869_RGMII_CLK_DELAY_INV;
>>>> +	ret = of_property_read_u32(of_node, "tx-internal-delay-ps",
>>>> +				   &dp83869->tx_id_delay);
>>>> +	if (!ret && dp83869->tx_id_delay > dp83869_internal_delay[delay_size]) {
>>>> +		phydev_err(phydev,
>>>> +			   "tx-internal-delay value of %u out of range\n",
>>>> +			   dp83869->tx_id_delay);
>>>> +		return -EINVAL;
>>>> +	}
>>> This is the kind of validation that I would be expecting from the PHY
>>> library to do, in fact, since you use Device Tree standard property, I
>>> would expect you only need to pass the maximum delay value and some
>>> storage for your array of delays.
>> Actually the PHY library will return either the 0th index if the value is to
>> small or the max index if the value is to large
>>
>> based on the array passed in so maybe this check is unnecessary.
> Hi Dan
>
> I'm not sure the helper is implementing the best behaviour. Rounded to
> the nearest when within the supported range is O.K. But if the request
> is outside the range, i would report an error.

Hope this email does not bounce.

> Any why is your PHY special, in that is does care about out of range
> delays, when others using new the new core helper don't?

We are not rounding to nearest here.  Basically the helper works to find 
the best match

If the delay passed in is less than or equal to the smallest delay then 
return the smallest delay index

If the delay passed in is greater then the largest delay then return the 
max delay index

Not sure what you mean about this PHY being special.  This helper is not 
PHY specific.

After I think about this more I am thinking a helper may be over kill 
here and the delay to setting should be done within the PHY driver itself

Dan

> 	Andrew

Powered by blists - more mailing lists