[RFC PATCH 3/6] phy: rockcip-pcie: reconstruct driver to support per-lane PHYs

[Shawn Lin]

Hi Jeffy,

On 2017/7/14 15:03, jeffy wrote:
> Hi Shawn,
> 
> On 07/14/2017 02:33 PM, Shawn Lin wrote:
>>>
>>>> +        return rk_phy->phys[0];
>>>> +    }
>>>> +}
>>>> +
>>>>   static inline void phy_wr_cfg(struct rockchip_pcie_phy *rk_phy,
>>>>                     u32 addr, u32 data)
>>>>   {
>>>> @@ -114,20 +139,55 @@ static inline u32 phy_rd_cfg(struct
>>>> rockchip_pcie_phy *rk_phy,
>>>>       return val;
>>>>   }
>>>> -static int rockchip_pcie_phy_power_off(struct phy *phy)
>>>> +static int rockchip_pcie_phy_common_power_off(struct phy *phy)
>>>>   {
>>>>       struct rockchip_pcie_phy *rk_phy = phy_get_drvdata(phy);
>>>>       int err = 0;
>>>> +    if (WARN_ON(!rk_phy->pwr_cnt))
>>>> +        return -EINVAL;
>>>> +
>>>> +    if (rk_phy->pwr_cnt > 0)
>>>
>>> This should be:
>>>
>>>     if (--rk_phy->pwr_cnt)
>>>
>>> Also, you technically might need locking, now that multiple phys (which
>>> each only have their own independent mutex) are accessing the same
>>> refcount. Or maybe just make this an atomic variable.
>>
>> Good catch!
> Sounds like we need something similar to phy-core.c's power_count and 
> init_count.

Probably, and I will look into it later.

> 
>>>> +
>>>>       return 0;
>>>>   }
>>>> +#define DECLARE_PHY_POWER_OFF_PER_LANE(id) \
>>>> +static int rockchip_pcie_lane##id##_phy_power_off(struct phy *phy) \
>>>
>>> What? All this macro magic (and duplicate generated functions) should
>>> not be necessary. You just need some per-phy data that keeps the index.
>>
>> I can't quite follow yours here. The only argument passing on to
>> the PHY APIs is 'struct phy *phy', and how could you trace the index
>> from it? The caller should save phy instead of 'rockchip_pcie_phy', in
>> which the per-phy data should be.
>>
>> Or could you kindly show me some example here:)
>>
> Maybe add a struct rockchip_pcie_phy_data for each phy, contains their 
> index and a pointer to the common struct rockchip_pcie_phy?

yes, I got Brian's point after reading it more times, and I almost
finish converting to per-lane data now..

> 
> 
> 
> 
>