Re: Re: [PATCH] ARM: sunxi: Add driver for sunxi usb phy

[Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]

Hi,

On 01/16/2014 08:07 AM, Chen-Yu Tsai wrote:
> Hi Hans,
>
> On Wed, Jan 15, 2014 at 11:48 PM, Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>> Hi,
>>
>>
>> On 01/15/2014 04:00 PM, Kishon Vijay Abraham I wrote:
>>>
>>> On Wednesday 15 January 2014 04:28 AM, Hans de Goede wrote:
> [...]
>>>> +static int sun4i_usb_phy_init(struct phy *_phy)
>>>> +{
>>>> +       struct sun4i_usb_phy *phy = phy_get_drvdata(_phy);
>>>> +       struct sun4i_usb_phy_data *data = to_sun4i_usb_phy_data(phy);
>>>> +       int ret;
>>>> +
>>>> +       ret = clk_prepare_enable(data->clk);
>>>> +       if (ret)
>>>> +               return ret;
>>>> +
>>>> +       ret = reset_control_deassert(phy->reset);
>>>> +       if (ret) {
>>>> +               clk_disable_unprepare(data->clk);
>>>> +               return ret;
>>>> +       }
>>>> +
>>>> +       /* Adjust PHY's magnitude and rate */
>>>> +       sun4i_usb_phy_write(phy, 0x20, 0x14, 5);
>>>
>>>
>>> No magic values. Use macros instead.
>>
>>
>> We don't have docs, these values come from the Android code (and the comment
>> above has been translated from Chinese). I can make up some random
>> macros for this, but seems counter-productive, it seems best to just leave
>> this as magic until the day we actually have documentation and thus can use
>> defines with the proper register names, etc.
>
> We have some names for the registers from Allwinner code:
> https://github.com/linux-sunxi/linux-sunxi/blob/lichee-3.0.8-sun4i/drivers/usb/sun4i_usb/usbc/usbc_phy.c#L39

Ah good catch, thanks. I'll use those in the next revision of the phy driver.

Regards,

Hans