Messages in this thread | | | Date | Mon, 13 Jul 2020 10:38:39 +0530 | From | Vinod Koul <> | Subject | Re: [PATCH v1 1/1] phy: tusb1210: use bitmasks to set VENDOR_SPECIFIC2 |
| |
On 01-07-20, 21:06, Liam Beguin wrote: > From: Liam Beguin <lvb@xiphos.com> > > Start by reading the content of the VENDOR_SPECIFIC2 register and update > each bit field based on device properties when defined. > > The use of bit masks prevents fields from overriding each other and > enables users to clear bits which are set by default, like datapolarity > in this instance. > > Signed-off-by: Liam Beguin <lvb@xiphos.com> > --- > drivers/phy/ti/phy-tusb1210.c | 29 ++++++++++++++++------------- > 1 file changed, 16 insertions(+), 13 deletions(-) > > diff --git a/drivers/phy/ti/phy-tusb1210.c b/drivers/phy/ti/phy-tusb1210.c > index d8d0cc11d187..a957655ebd36 100644 > --- a/drivers/phy/ti/phy-tusb1210.c > +++ b/drivers/phy/ti/phy-tusb1210.c > @@ -13,9 +13,9 @@ > #include <linux/phy/ulpi_phy.h> > > #define TUSB1210_VENDOR_SPECIFIC2 0x80 > -#define TUSB1210_VENDOR_SPECIFIC2_IHSTX_SHIFT 0 > -#define TUSB1210_VENDOR_SPECIFIC2_ZHSDRV_SHIFT 4 > -#define TUSB1210_VENDOR_SPECIFIC2_DP_SHIFT 6 > +#define TUSB1210_VENDOR_SPECIFIC2_IHSTX_MASK 0x0f > +#define TUSB1210_VENDOR_SPECIFIC2_ZHSDRV_MASK 0x30
Why not be better and use GENMASK(3, 0) and GENMASK(5, 4) for these
> +#define TUSB1210_VENDOR_SPECIFIC2_DP_MASK BIT(6) > > struct tusb1210 { > struct ulpi *ulpi; > @@ -118,22 +118,25 @@ static int tusb1210_probe(struct ulpi *ulpi) > * diagram optimization and DP/DM swap. > */ > > + reg = ulpi_read(ulpi, TUSB1210_VENDOR_SPECIFIC2); > + > /* High speed output drive strength configuration */ > - device_property_read_u8(&ulpi->dev, "ihstx", &val); > - reg = val << TUSB1210_VENDOR_SPECIFIC2_IHSTX_SHIFT; > + if (!device_property_read_u8(&ulpi->dev, "ihstx", &val)) > + reg = (reg & ~TUSB1210_VENDOR_SPECIFIC2_IHSTX_MASK) | > + (val & TUSB1210_VENDOR_SPECIFIC2_IHSTX_MASK);
It would be better to use a helper to do this
> > /* High speed output impedance configuration */ > - device_property_read_u8(&ulpi->dev, "zhsdrv", &val); > - reg |= val << TUSB1210_VENDOR_SPECIFIC2_ZHSDRV_SHIFT; > + if (!device_property_read_u8(&ulpi->dev, "zhsdrv", &val)) > + reg = (reg & ~TUSB1210_VENDOR_SPECIFIC2_ZHSDRV_MASK) | > + (val & TUSB1210_VENDOR_SPECIFIC2_ZHSDRV_MASK); > > /* DP/DM swap control */ > - device_property_read_u8(&ulpi->dev, "datapolarity", &val); > - reg |= val << TUSB1210_VENDOR_SPECIFIC2_DP_SHIFT; > + if (!device_property_read_u8(&ulpi->dev, "datapolarity", &val)) > + reg = (reg & ~TUSB1210_VENDOR_SPECIFIC2_DP_MASK) | > + (val & TUSB1210_VENDOR_SPECIFIC2_DP_MASK); > > - if (reg) { > - ulpi_write(ulpi, TUSB1210_VENDOR_SPECIFIC2, reg); > - tusb->vendor_specific2 = reg; > - } > + ulpi_write(ulpi, TUSB1210_VENDOR_SPECIFIC2, reg); > + tusb->vendor_specific2 = reg; > > tusb->phy = ulpi_phy_create(ulpi, &phy_ops); > if (IS_ERR(tusb->phy)) > > base-commit: cd77006e01b3198c75fb7819b3d0ff89709539bb
fatal: bad object cd77006e01b3198c75fb7819b3d0ff89709539bb in linux-phy.git
-- ~Vinod
| |