[PATCH 3/6] phy: renesas: Add RZ/G2L usb phy control driver

[Biju Das]

Hi Vinod,

Thanks for the feedback.

> Subject: Re: [PATCH 3/6] phy: renesas: Add RZ/G2L usb phy control driver
> 
> On 11-06-21, 14:46, Biju Das wrote:
> > +static int rzg2l_usbphycontrol_probe(struct platform_device *pdev) {
> > +	struct device *dev = &pdev->dev;
> > +	struct rzg2l_usbphycontrol_drv *drv;
> > +	struct phy_provider *provider;
> > +	u32 val;
> > +	int n;
> > +
> > +	if (!dev->of_node) {
> > +		dev_err(dev, "device tree not found\n");
> > +		return -EINVAL;
> > +	}
> 
> why do you think this would happen?

Not needed. Will take out.

> 
> > +
> > +	drv = devm_kzalloc(dev, sizeof(*drv), GFP_KERNEL);
> > +	if (!drv)
> > +		return -ENOMEM;
> > +
> > +	drv->base = devm_platform_ioremap_resource(pdev, 0);
> > +	if (IS_ERR(drv->base))
> > +		return PTR_ERR(drv->base);
> > +
> > +	for (n = 0; n < NUM_PORTS; n++) {
> > +		struct rzg2l_phyctrl *phy = &drv->phyctrl[n];
> > +
> > +		phy->phy = devm_phy_create(dev, NULL, &rzg2l_usbphyctrl_ops);
> > +		if (IS_ERR(phy->phy)) {
> > +			dev_err(dev, "Failed to create USBPHY Control\n");
> > +			return PTR_ERR(phy->phy);
> > +		}
> > +
> > +		if (n == 1)
> > +			phy->phy_reset_port_mask = PHY_RESET_PORT2;
> 
> this looks inverted, should this logically not be:
Agreed. Will change this in v2.

>                 if (n == 0)
>                         phy->phy_reset_port_mask = PHY_RESET_PORT1; ?
> 
> > +		else
> > +			phy->phy_reset_port_mask = PHY_RESET_PORT1;
> > +
> > +		phy->drv = drv;
> > +		phy_set_drvdata(phy->phy, phy);
> > +	};
> > +
> > +	provider = devm_of_phy_provider_register(dev,
> > +						 rzg2l_usbphycontrol_xlate);
> 
> single line pls
OK.

Regards,
Biju

> 
> --
> ~Vinod