All of lore.kernel.org
 help / color / mirror / Atom feed
From: lyz@rock-chips.com (Yunzhi Li)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v3 1/5] phy: add a driver for the Rockchip SoC internal USB2.0 PHY
Date: Mon, 08 Dec 2014 19:04:20 +0800	[thread overview]
Message-ID: <548585B4.107@rock-chips.com> (raw)
In-Reply-To: <548575F0.4090909@ti.com>

Hi Kishon :

On 2014/12/8 17:57, Kishon Vijay Abraham I wrote:
> Hi,
>
> On Monday 08 December 2014 03:16 PM, Yunzhi Li wrote:
>> This patche to add a generic PHY driver for ROCKCHIP usb PHYs,
> %s/patche/patch
Sorry for this typo.
>> +#include <linux/reset.h>
>> +#include <linux/regmap.h>
>> +#include <linux/mfd/syscon.h>
>> +
>> +#define ROCKCHIP_RK3288_UOC(n)	(0x320 + n * 0x14)
>> +
>> +#define SIDDQ_MSK		BIT(13 + 16)
> This doesn't look correct. Won't this mask bit no:29?
The higher 16-bit of this register is used for write-protect, only if 
BIT(13 + 16)
set to 1 the BIT(13) can be written. I will add some comments here in 
the next
version, to prevent others confused. ok ?

>> [...]
>>
>> +static int rockchip_usb_phy_probe(struct platform_device *pdev)
>> +{
>> +	struct device *dev = &pdev->dev;
>> +	struct rockchip_usb_phy *rk_phy;
>> +	struct rockchip_usb_phy *phy_array;
>> +	struct phy_provider *phy_provider;
>> +	struct regmap *grf;
>> +	char clk_name[16];
>> +	int i;
>> +
>> +	grf = syscon_regmap_lookup_by_phandle(dev->of_node, "rockchip,grf");
>> +	if (IS_ERR(grf)) {
>> +		dev_err(&pdev->dev, "Missing rockchip,grf property\n");
>> +		return PTR_ERR(grf);
>> +	}
>> +
>> +	phy_array = devm_kzalloc(dev, RK3288_NUM_PHYS * sizeof(*rk_phy),
>> +				 GFP_KERNEL);
>> +	if (!phy_array)
>> +		return -ENOMEM;
>> +
>> +	for (i = 0; i < RK3288_NUM_PHYS; i++) {
> Don't hardcode NUM_PHYS. All this has to come from dt. Model each PHYs as
> subnode of the phy provider node and use it here.
I got it, it should be like this, model each PHYs as sub node and use 
of_get_child_count() to get the number of phys.

WARNING: multiple messages have this Message-ID (diff)
From: Yunzhi Li <lyz@rock-chips.com>
To: Kishon Vijay Abraham I <kishon@ti.com>,
	heiko@sntech.de, dianders@chromium.org, romain.perier@gmail.com
Cc: olof@lixom.net, huangtao@rock-chips.com, zyw@rock-chips.com,
	cf@rock-chips.com, linux-rockchip@lists.infradead.org,
	Grant Likely <grant.likely@linaro.org>,
	Rob Herring <robh+dt@kernel.org>,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org
Subject: Re: [PATCH v3 1/5] phy: add a driver for the Rockchip SoC internal USB2.0 PHY
Date: Mon, 08 Dec 2014 19:04:20 +0800	[thread overview]
Message-ID: <548585B4.107@rock-chips.com> (raw)
In-Reply-To: <548575F0.4090909@ti.com>

Hi Kishon :

On 2014/12/8 17:57, Kishon Vijay Abraham I wrote:
> Hi,
>
> On Monday 08 December 2014 03:16 PM, Yunzhi Li wrote:
>> This patche to add a generic PHY driver for ROCKCHIP usb PHYs,
> %s/patche/patch
Sorry for this typo.
>> +#include <linux/reset.h>
>> +#include <linux/regmap.h>
>> +#include <linux/mfd/syscon.h>
>> +
>> +#define ROCKCHIP_RK3288_UOC(n)	(0x320 + n * 0x14)
>> +
>> +#define SIDDQ_MSK		BIT(13 + 16)
> This doesn't look correct. Won't this mask bit no:29?
The higher 16-bit of this register is used for write-protect, only if 
BIT(13 + 16)
set to 1 the BIT(13) can be written. I will add some comments here in 
the next
version, to prevent others confused. ok ?

>> [...]
>>
>> +static int rockchip_usb_phy_probe(struct platform_device *pdev)
>> +{
>> +	struct device *dev = &pdev->dev;
>> +	struct rockchip_usb_phy *rk_phy;
>> +	struct rockchip_usb_phy *phy_array;
>> +	struct phy_provider *phy_provider;
>> +	struct regmap *grf;
>> +	char clk_name[16];
>> +	int i;
>> +
>> +	grf = syscon_regmap_lookup_by_phandle(dev->of_node, "rockchip,grf");
>> +	if (IS_ERR(grf)) {
>> +		dev_err(&pdev->dev, "Missing rockchip,grf property\n");
>> +		return PTR_ERR(grf);
>> +	}
>> +
>> +	phy_array = devm_kzalloc(dev, RK3288_NUM_PHYS * sizeof(*rk_phy),
>> +				 GFP_KERNEL);
>> +	if (!phy_array)
>> +		return -ENOMEM;
>> +
>> +	for (i = 0; i < RK3288_NUM_PHYS; i++) {
> Don't hardcode NUM_PHYS. All this has to come from dt. Model each PHYs as
> subnode of the phy provider node and use it here.
I got it, it should be like this, model each PHYs as sub node and use 
of_get_child_count() to get the number of phys.

  reply	other threads:[~2014-12-08 11:04 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-08  9:46 [PATCH v3 0/5] Yunzhi Li
2014-12-08  9:46 ` Yunzhi Li
2014-12-08  9:46 ` [PATCH v3 1/5] phy: add a driver for the Rockchip SoC internal USB2.0 PHY Yunzhi Li
2014-12-08  9:46   ` Yunzhi Li
2014-12-08  9:57   ` Kishon Vijay Abraham I
2014-12-08  9:57     ` Kishon Vijay Abraham I
2014-12-08  9:57     ` Kishon Vijay Abraham I
2014-12-08 11:04     ` Yunzhi Li [this message]
2014-12-08 11:04       ` Yunzhi Li
     [not found] ` <1418031988-2700-1-git-send-email-lyz-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
2014-12-08  9:46   ` [PATCH v3 2/5] Documentation: bindings: add doc for the Rockchip usb PHY Yunzhi Li
2014-12-08  9:46     ` Yunzhi Li
2014-12-08  9:46 ` [PATCH v3 3/5] usb: dwc2: add generic PHY framework support for dwc2 usb controler platform driver Yunzhi Li
2014-12-08 20:55   ` Paul Zimmerman
2014-12-08  9:46 ` [PATCH v3 4/5] ARM: dts: add rk3288 usb PHY Yunzhi Li
2014-12-08  9:46   ` Yunzhi Li

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=548585B4.107@rock-chips.com \
    --to=lyz@rock-chips.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.