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

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

On Thu, Jan 15, 2026 at 05:35:48PM +0530, Vinod Koul wrote:
> On 15-01-26, 14:42, Jiayu Du wrote:
> 
> > +static struct phy *k230_usb_phy_xlate(struct device *dev,
> > +				      const struct of_phandle_args *args)
> > +{
> > +	struct k230_usb_phy_global *global = dev_get_drvdata(dev);
> > +	struct k230_usb_phy_instance *phy_inst;
> > +	struct phy *phy;
> > +
> > +	if (args->args[0] >= MAX_PHYS)
> > +		return ERR_PTR(-EINVAL);
> > +
> > +	phy_inst = devm_kzalloc(dev, sizeof(*phy_inst), GFP_KERNEL);
> > +	if (!phy_inst)
> > +		return ERR_PTR(-ENOMEM);
> > +
> > +	phy_inst->global = global;
> > +	phy_inst->index = args->args[0];
> > +
> > +	phy = devm_phy_create(dev, NULL, &k230_usb_phy_ops);
> > +	if (IS_ERR(phy))
> > +		return ERR_PTR(PTR_ERR(phy));
> > +
> > +	phy_set_drvdata(phy, phy_inst);
> 
> This seems wrong place, why is this not done in the driver probe?

You are right, creating phy instances in the xlate function is not
the right place. I will move the allocation and devm_phy_creat to
the probe phase to create both instances.

> > +	global->reg_test_offset[0] = 0x70;
> > +	global->reg_ctl_offset[0] = 0xb0;
> > +	global->reg_test_offset[1] = 0x90;
> > +	global->reg_ctl_offset[1] = 0xb8;
> 
> Where are these magic values coming from?

These offsets are from K230 Technical Reference Manual section 12.6.4.2.
The TRM is here[1]. I will define them as macros with descriptive names.

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]

I will send v2 with these changes soon.

Regards,
Jiayu Du