On Tue, Jun 14, 2016 at 6:14 PM, Frank Wang <frank.wang@xxxxxxxxxxxxxx> wrote: > Hi Heiko & Guenter, > > > On 2016/6/14 22:00, Heiko Stübner wrote: >> >> Am Dienstag, 14. Juni 2016, 06:50:31 schrieb Guenter Roeck: >>> >>> On Tue, Jun 14, 2016 at 6:27 AM, Heiko Stübner <heiko@xxxxxxxxx> wrote: >>>> >>>> Am Montag, 13. Juni 2016, 10:10:10 schrieb Frank Wang: >>>>> >>>>> The newer SoCs (rk3366, rk3399) take a different usb-phy IP block >>>>> than rk3288 and before, and most of phy-related registers are also >>>>> different from the past, so a new phy driver is required necessarily. >>>>> >>>>> Signed-off-by: Frank Wang <frank.wang@xxxxxxxxxxxxxx> >>>>> --- >> >> [...] >> >>>>> +static int rockchip_usb2phy_init(struct phy *phy) >>>>> +{ >>>>> + struct rockchip_usb2phy_port *rport = phy_get_drvdata(phy); >>>>> + struct rockchip_usb2phy *rphy = dev_get_drvdata(phy->dev.parent); >>>>> + int ret; >>>>> + >>>>> >>>> if (!rport->port_cfg) >>>> return 0; >>>> >>>> Otherwise the currently empty otg-port will cause null-pointer >>>> dereferences >>>> when it gets assigned in the devicetree already. >>> >>> Not really, at least not here - that port should not have port_id set >>> to USB2PHY_PORT_HOST. >>> >>> Does it even make sense to instantiate the otg port ? Is it going to >>> do anything without port configuration ? >> >> Ok, that would be the other option - not creating the phy in the driver. > > > Well, I will put this conditional inside *_host_port_init(), if it is an > empty, the phy-device should not be created. > Something like the following: > > --- a/drivers/phy/phy-rockchip-inno-usb2.c > +++ b/drivers/phy/phy-rockchip-inno-usb2.c > @@ -483,9 +483,13 @@ static int rockchip_usb2phy_host_port_init(struct > rockchip_usb2phy *rphy, > { > int ret; > > - rport->port_id = USB2PHY_PORT_HOST; > rport->port_cfg = &rphy->phy_cfg->port_cfgs[USB2PHY_PORT_HOST]; > + if (!rport->port_cfg) { > + dev_err(rphy->dev, "no host port-config provided.\n"); > + return -EINVAL; > + } This would never be NULL. At issue is that you don't assign port_cfg if the port is _not_ a host port. Guenter > > + rport->port_id = USB2PHY_PORT_HOST; > >> Or from what I've seen, handling it as similar to the host-port should >> work >> initially as well most likely, supplying the additional otg-parts later >> on. > > > @Guenter, just as Heiko said, the otg-parts is not ready now, it will be > supplied later. > > > BR. > Frank > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html