Hi, On Friday 01 September 2017 08:11 AM, Yoshihiro Shimoda wrote: > The has_otg on previous code has the two meaning: > - The channel has dedicated otg pins (ID, VBUS). > - The channel can swap the role via sysfs. > > However, some SoCs (e.g. R-Car D3) doesn't have such dedicated pins, > but the SoCs can swap the role. So, this patch split the two meaning > of has_otg as "has dedicated otg pins" and adds a new value > "can_role_swap". For now, the behavior is the same as before.
Might not be directly related to $patch, but in general certain cleanups can be done.. > > Signed-off-by: Yoshihiro Shimoda <[email protected]> > --- > drivers/phy/renesas/phy-rcar-gen3-usb2.c | 61 > ++++++++++++++++++++++---------- > 1 file changed, 42 insertions(+), 19 deletions(-) > > diff --git a/drivers/phy/renesas/phy-rcar-gen3-usb2.c > b/drivers/phy/renesas/phy-rcar-gen3-usb2.c > index e00e99a..4ea9902 100644 > --- a/drivers/phy/renesas/phy-rcar-gen3-usb2.c > +++ b/drivers/phy/renesas/phy-rcar-gen3-usb2.c > @@ -87,7 +87,8 @@ struct rcar_gen3_chan { > struct regulator *vbus; > struct work_struct work; > bool extcon_host; > - bool has_otg; > + bool has_otg; /* has dedicated pins (ID, VBUS) */ > + bool can_role_swap; > }; > > static void rcar_gen3_phy_usb2_work(struct work_struct *work) > @@ -219,14 +220,10 @@ static bool rcar_gen3_is_host(struct rcar_gen3_chan *ch) > return !(readl(ch->base + USB2_COMMCTRL) & USB2_COMMCTRL_OTG_PERI); > } > > -static ssize_t role_store(struct device *dev, struct device_attribute *attr, > - const char *buf, size_t count) > +static ssize_t rcar_gen3_otg_change_role(struct rcar_gen3_chan *ch, > + bool new_mode_is_host) replace new_mode_is_host with enum mode. > { > - struct rcar_gen3_chan *ch = dev_get_drvdata(dev); > - bool is_b_device, is_host, new_mode_is_host; > - > - if (!ch->has_otg || !ch->phy->init_count) > - return -EIO; > + bool is_b_device, is_host; > > /* > * is_b_device: true is B-Device. false is A-Device. > @@ -234,12 +231,6 @@ static ssize_t role_store(struct device *dev, struct > device_attribute *attr, > */ > is_b_device = rcar_gen3_check_id(ch); > is_host = rcar_gen3_is_host(ch); > - if (!strncmp(buf, "host", strlen("host"))) > - new_mode_is_host = true; > - else if (!strncmp(buf, "peripheral", strlen("peripheral"))) > - new_mode_is_host = false; > - else > - return -EINVAL; > > /* If current and new mode is the same, this returns the error */ > if (is_host == new_mode_is_host) > @@ -257,6 +248,32 @@ static ssize_t role_store(struct device *dev, struct > device_attribute *attr, > rcar_gen3_init_for_peri(ch); > } > > + return 0; > +} > + > +static ssize_t role_store(struct device *dev, struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + struct rcar_gen3_chan *ch = dev_get_drvdata(dev); > + bool new_mode_is_host; > + ssize_t ret = -EIO; > + > + if (!ch->phy->init_count) > + return -EIO; > + > + if (!strncmp(buf, "host", strlen("host"))) > + new_mode_is_host = true; > + else if (!strncmp(buf, "peripheral", strlen("peripheral"))) > + new_mode_is_host = false; set the enum mode here. > + else > + return -EINVAL; > + > + if (ch->has_otg) > + ret = rcar_gen3_otg_change_role(ch, new_mode_is_host); Aren't you using sysfs to change the role here? > + > + if (ret < 0) > + return ret; > + > return count; > } > > @@ -264,12 +281,17 @@ static ssize_t role_show(struct device *dev, struct > device_attribute *attr, > char *buf) > { > struct rcar_gen3_chan *ch = dev_get_drvdata(dev); > + bool is_host; > > - if (!ch->has_otg || !ch->phy->init_count) > + if (!ch->phy->init_count) > return -EIO; > > - return sprintf(buf, "%s\n", rcar_gen3_is_host(ch) ? "host" : > - "peripheral"); > + if (ch->has_otg) > + is_host = rcar_gen3_is_host(ch); > + else > + return -EIO; > + > + return sprintf(buf, "%s\n", is_host ? "host" : "peripheral"); > } > static DEVICE_ATTR_RW(role); > > @@ -427,6 +449,7 @@ static int rcar_gen3_phy_usb2_probe(struct > platform_device *pdev) > int ret; > > channel->has_otg = true; > + channel->can_role_swap = true; Can both has_otg and can_role_swap be true? Thanks Kishon
