[U-Boot] [uboot] [PATCH 2/2] net: phy: ti: Allow the driver to be more configurable

[Dan Murphy]

On 04/05/2016 01:30 PM, Dan Murphy wrote:
> On 04/05/2016 12:36 PM, Michal Simek wrote:
>> On 5.4.2016 19:18, Dan Murphy wrote:
>>> Michal
>>>
>>> On 04/05/2016 12:01 PM, Michal Simek wrote:
>>>> On 5.4.2016 17:05, Dan Murphy wrote:
>>>>> Not all devices use the same internal delay or fifo depth.
>>>>> Add the ability to set the internal delay for rx or tx and the
>>>>> fifo depth via the devicetree.  If the value is not set in the
>>>>> devicetree then set the delay to the default.
>>>>>
>>>>> If devicetree is not used then use the default defines within the
>>>>> driver.
>>>>>
>>>>> Signed-off-by: Dan Murphy <dmurphy at ti.com>
>>>>> ---
>>>>>
>>>>> RFC->v1 - Added devicetree support - https://patchwork.ozlabs.org/patch/604113/
>>>>>
>>>>>  drivers/net/phy/ti.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++------
>>>>>  1 file changed, 73 insertions(+), 8 deletions(-)
>>>>>
>>>>> diff --git a/drivers/net/phy/ti.c b/drivers/net/phy/ti.c
>>>>> index c3912d5..e91a6ed 100644
>>>>> --- a/drivers/net/phy/ti.c
>>>>> +++ b/drivers/net/phy/ti.c
>>>>> @@ -6,6 +6,14 @@
>>>>>   */
>>>>>  #include <common.h>
>>>>>  #include <phy.h>
>>>>> +#include <linux/compat.h>
>>>>> +#include <malloc.h>
>>>>> +
>>>>> +#include <fdtdec.h>
>>>>> +#include <dm.h>
>>>>> +#include <dt-bindings/net/ti-dp83867.h>
>>>>> +
>>>>> +DECLARE_GLOBAL_DATA_PTR;
>>>>>  
>>>>>  /* TI DP83867 */
>>>>>  #define DP83867_DEVADDR		0x1f
>>>>> @@ -57,6 +65,17 @@
>>>>>  #define MII_MMD_CTRL_INCR_RDWT	0x8000 /* post increment on reads & writes */
>>>>>  #define MII_MMD_CTRL_INCR_ON_WT	0xC000 /* post increment on writes only */
>>>>>  
>>>>> +/* User setting - can be taken from DTS */
>>>>> +#define DEFAULT_RX_ID_DELAY	DP83867_RGMIIDCTL_2_25_NS
>>>>> +#define DEFAULT_TX_ID_DELAY	DP83867_RGMIIDCTL_2_75_NS
>>>>> +#define DEFAULT_FIFO_DEPTH	DP83867_PHYCR_FIFO_DEPTH_4_B_NIB
>>>>> +
>>>>> +struct dp83867_private {
>>>>> +	int rx_id_delay;
>>>>> +	int tx_id_delay;
>>>>> +	int fifo_depth;
>>>>> +};
>>>>> +
>>>>>  /**
>>>>>   * phy_read_mmd_indirect - reads data from the MMD registers
>>>>>   * @phydev: The PHY device bus
>>>>> @@ -134,16 +153,58 @@ static inline bool phy_interface_is_rgmii(struct phy_device *phydev)
>>>>>  		phydev->interface <= PHY_INTERFACE_MODE_RGMII_TXID;
>>>>>  }
>>>>>  
>>>>> -/* User setting - can be taken from DTS */
>>>>> -#define RX_ID_DELAY	8
>>>>> -#define TX_ID_DELAY	0xa
>>>>> -#define FIFO_DEPTH	1
>>>>> +#if defined(CONFIG_DM_ETH)
>>>>> +/**
>>>>> + * dp83867_data_init - Convenience function for setting PHY specific data
>>>>> + *
>>>>> + * @phydev: the phy_device struct
>>>>> + */
>>>>> +static int dp83867_of_init(struct phy_device *phydev)
>>>>> +{
>>>>> +	struct dp83867_private *dp83867 = phydev->priv;
>>>>> +	struct udevice *dev = phydev->dev;
>>>>> +
>>>>> +	dp83867->rx_id_delay = fdtdec_get_uint(gd->fdt_blob, dev->of_offset,
>>>>> +				 "ti,rx_int_delay", DEFAULT_RX_ID_DELAY);
>>>>> +
>>>>> +	dp83867->tx_id_delay = fdtdec_get_uint(gd->fdt_blob, dev->of_offset,
>>>>> +				 "ti,tx_int_delay", DEFAULT_TX_ID_DELAY);
>>>>> +
>>>> this is not aligned with the binding you sent.
>>>> ti,rx-internal-delay
>>>> and
>>>> ti,tx-internal-delay
>>> You are right I will fix it and then make the same change in the kernel
>> Why not just follow linux binding?
>> Changing binding in kernel will end up in deprecated binding for nothing.
> What specific Linux binding are you referring to?
>
> I don't see this binding getting changed as the internal delay and fifo depth
> are board configurable options and I don't for see the register map changing for this
> device.

I see it now.  Will submit v3 of the driver but I will wait for additional comments
before sending tomorrow.


Dan

> Dan
>
>
>> Thanks,
>> Michal
>>
>>
>


-- 
------------------
Dan Murphy