Re: [PATCH 4/5] phy: usb: Add driver for Canaan K230 USB 2.0 PHY

[Jiayu Du <jiayu.riscv@isrc.iscas.ac.cn>]

On Thu, Jan 01, 2026 at 04:18:01PM +0530, Vinod Koul wrote:
> On 30-12-25, 10:37, Jiayu Du wrote:
> > Add driver for the USB 2.0 PHY in Canaan K230 SoC, which supports PHY
> > initialization, power management and USB mode switching.
> > 
> > Add Kconfig/Makefile under drivers/phy/canaan/.
> > 
> > Signed-off-by: Jiayu Du <jiayu.riscv@isrc.iscas.ac.cn>
...
> > +#define TEST_CTL3_OFFSET	0x0C
> 
> Lowercase hex values please.. do you need a test register :-)

Sorry, I will convert the hex to lowercase.

In the TRM manual, the registers are named TEST_CTL and they are
used to describe the otg0 phy port control. The TRM manual is here[1].
The description of this register is located on page 1015.

Therefore, I have retained the names as stated in the TRM manual.

Link:
https://kendryte-download.canaan-creative.com/developer/k230/HDK/K230%E7%A1%AC%E4%BB%B6%E6%96%87%E6%A1%A3/K230_Technical_Reference_Manual_V0.3.1_20241118.pdf
[1]

> > +	FIELD_PREP(USB_CTL0_PLLPTUNE_MASK, 0xC) | \
> 
> lower hex here and rest

I will fix it in v2.

> 
> > +	ret = regmap_update_bits(phy->regmap, phy->reg_test_offset +
> > +				 TEST_CTL3_OFFSET, val, val);
> 
> so we are writing to a test register..?

As I mentioned above, this is actually otg0 phy port control
register.

> > +	int ret;
> > +	u32 offset;
> > +	struct regmap *regmap;
> > +	struct phy *generic_phy;
> > +	struct k230_usb_phy *phy;
> > +	struct phy_provider *provider;
> > +	struct device *dev = &pdev->dev;
> 
> reverse christmas  tree order would look better...

I will fix it in v2.

> > +
> > +
> 
> why two lines...?

I will fix it in v2.

> 
> > +MODULE_LICENSE("GPL");
> > -- 
> > 2.52.0
> 
> -- 
> ~Vinod

Regards,
Jiayu Du