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
