Hi Kishon,
On 2016/1/4 15:45, Kishon Vijay Abraham I wrote:
> Hi,
>
> On Tuesday 29 December 2015 07:22 AM, Shawn Lin wrote:
>> This patch to add a generic PHY driver for ROCKCHIP eMMC PHY.
[...]
>> +
>> +struct rockchip_emmc_phy {
>> + unsigned int reg_offset;
>> + struct regmap *reg_base;
>> + struct phy *phy;
>
> The phy looks unnecessary.
right, got it.
>> + bool state;
>
> hmm.. I want to have some sort of state machine in phy core so that individual
> PHY drivers don't have to maintain the state. However I'm not sure if all the
> PHY's will require such mechanism.
>
In general, phy core is the best place to maintain it.
Presumably, phy driver maintainer is much likely to
maintain the corresponding caller driver at the same time, so he/she
should be in charge of maintaining the on/off, init/exit pairs. From
this point, it doesn't need the state machine. But, I'm not sure if the
caller driver will always keep the on/off pairs correct.
Anyway, I will remove it from this driver. And may we request a RFC
for all sub-phy drivers to discuss this issue if we want to add the
state machine into phy core. How about?
>> +};
>> +
>> +static int rockchip_emmc_phy_power(struct rockchip_emmc_phy *rk_phy,
[...]
>> +}
>> +
>> +static int rockchip_emmc_phy_init(struct phy *phy)
>> +{
>> + rockchip_emmc_phy_power_on(phy);
>
> do only phy initialization here, power on can be done later.
yep.
>> + return 0;
>> +}
>> +
>> +static int rockchip_emmc_phy_exit(struct phy *phy)
>> +{
>> + rockchip_emmc_phy_power_off(phy);
>
> same here.
>
> Thanks
> Kishon
>
>
>
--
Best Regards
Shawn Lin
