From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1753162AbcADIgr (ORCPT <rfc822;w@1wt.eu>);
	Mon, 4 Jan 2016 03:36:47 -0500
Received: from lucky1.263xmail.com ([211.157.147.131]:37148 "EHLO
	lucky1.263xmail.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1751521AbcADIgp (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Mon, 4 Jan 2016 03:36:45 -0500
X-263anti-spam: KSV:0;
X-MAIL-GRAY: 1
X-MAIL-DELIVERY: 0
X-KSVirus-check: 0
X-ABS-CHECKED: 4
X-ADDR-CHECKED: 0
X-RL-SENDER: shawn.lin@rock-chips.com
X-FST-TO: linux-rockchip@lists.infradead.org
X-SENDER-IP: 58.22.7.114
X-LOGIN-NAME: shawn.lin@rock-chips.com
X-UNIQUE-TAG: <7f13314a620c04a9df7b8684e2103e18>
X-ATTACHMENT-NUM: 0
X-DNS-TYPE: 0
Subject: Re: [PATCH 2/2] phy: add a driver for the Rockchip SoC internal eMMC
 PHY
To: Kishon Vijay Abraham I <kishon@ti.com>
References: <1451353928-23973-1-git-send-email-shawn.lin@rock-chips.com>
 <568A230F.3000605@ti.com>
Cc: shawn.lin@rock-chips.com, linux-kernel@vger.kernel.org,
        linux-rockchip@lists.infradead.org
From: Shawn Lin <shawn.lin@rock-chips.com>
Message-ID: <568A2F0E.8070608@rock-chips.com>
Date: Mon, 4 Jan 2016 16:36:30 +0800
User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:38.0) Gecko/20100101
 Thunderbird/38.5.0
MIME-Version: 1.0
In-Reply-To: <568A230F.3000605@ti.com>
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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