[PATCH v2 1/1] phy: tusb1210: use bitmasks to set VENDOR_SPECIFIC2

[PATCH v2 1/1] phy: tusb1210: use bitmasks to set VENDOR_SPECIFIC2

[Liam Beguin]

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 | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/phy/ti/phy-tusb1210.c b/drivers/phy/ti/phy-tusb1210.c
index d8d0cc11d187..35baeb9deeee 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	GENMASK(3, 0)
+#define TUSB1210_VENDOR_SPECIFIC2_ZHSDRV_MASK	GENMASK(5, 4)
+#define TUSB1210_VENDOR_SPECIFIC2_DP_MASK	BIT(6)
 
 struct tusb1210 {
 	struct ulpi *ulpi;
@@ -118,22 +118,22 @@ 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 = set_mask_bits(&reg, TUSB1210_VENDOR_SPECIFIC2_IHSTX_MASK, val);
 
 	/* 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 = set_mask_bits(&reg, TUSB1210_VENDOR_SPECIFIC2_ZHSDRV_MASK, val);
 
 	/* 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 = set_mask_bits(&reg, TUSB1210_VENDOR_SPECIFIC2_DP_MASK, val);
 
-	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: 8f3d9f354286745c751374f5f1fcafee6b3f3136
-- 
2.27.0

Re: [PATCH v2 1/1] phy: tusb1210: use bitmasks to set VENDOR_SPECIFIC2

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 1312 bytes --]

Hi Liam,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on 8f3d9f354286745c751374f5f1fcafee6b3f3136]

url:    https://github.com/0day-ci/linux/commits/Liam-Beguin/phy-tusb1210-use-bitmasks-to-set-VENDOR_SPECIFIC2/20200715-003517
base:    8f3d9f354286745c751374f5f1fcafee6b3f3136
config: parisc-allyesconfig (attached as .config)
compiler: hppa-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=parisc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   hppa-linux-ld: drivers/phy/ti/phy-tusb1210.o: in function `tusb1210_probe':
>> (.text+0x2f4): undefined reference to `__cmpxchg_called_with_bad_pointer'
>> hppa-linux-ld: (.text+0x324): undefined reference to `__cmpxchg_called_with_bad_pointer'
   hppa-linux-ld: (.text+0x354): undefined reference to `__cmpxchg_called_with_bad_pointer'

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 62183 bytes --]

Re: [PATCH v2 1/1] phy: tusb1210: use bitmasks to set VENDOR_SPECIFIC2

[Liam Beguin]

On Wed Jul 15, 2020 at 7:48 AM kernel test robot wrote:
> Hi Liam,
> 
> Thank you for the patch! Yet something to improve:
> 
> [auto build test ERROR on 8f3d9f354286745c751374f5f1fcafee6b3f3136]
> 
> url:    https://github.com/0day-ci/linux/commits/Liam-Beguin/phy-tusb1210-use-bitmasks-to-set-VENDOR_SPECIFIC2/20200715-003517
> base:    8f3d9f354286745c751374f5f1fcafee6b3f3136
> config: parisc-allyesconfig (attached as .config)
> compiler: hppa-linux-gcc (GCC) 9.3.0
> reproduce (this is a W=1 build):
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # save the attached .config to linux build tree
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=parisc 
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> 
> All errors (new ones prefixed by >>):
> 
>    hppa-linux-ld: drivers/phy/ti/phy-tusb1210.o: in function `tusb1210_probe':
> >> (.text+0x2f4): undefined reference to `__cmpxchg_called_with_bad_pointer'
> >> hppa-linux-ld: (.text+0x324): undefined reference to `__cmpxchg_called_with_bad_pointer'
>    hppa-linux-ld: (.text+0x354): undefined reference to `__cmpxchg_called_with_bad_pointer'
> 

Hi,

I sent another patch[1] to try to address this.

The issue seems to come from the fact that the parisc architecture
doesn't support cmpxchg on u8 pointers. cmpxchg is called by
set_mask_bits.

Thanks,
Liam

[1] https://lore.kernel.org/patchwork/patch/1272617/

> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

Re: [PATCH v2 1/1] phy: tusb1210: use bitmasks to set VENDOR_SPECIFIC2

[Liam Beguin]

Hi Vinod,

On Sat, Jul 18, 2020 at 4:18 PM Liam Beguin <liambeguin@gmail.com> wrote:
>
>
> Hi,
>
> I sent another patch[1] to try to address this.
>
> The issue seems to come from the fact that the parisc architecture
> doesn't support cmpxchg on u8 pointers. cmpxchg is called by
> set_mask_bits.
>

This patch has been merged.

> Thanks,
> Liam
>
> [1] https://lore.kernel.org/patchwork/patch/1272617/
>
> > ---
> > 0-DAY CI Kernel Test Service, Intel Corporation
> > https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

Did you get a change to look at the initial tusb1210 patch?

Thanks,
Liam

Re: [PATCH v2 1/1] phy: tusb1210: use bitmasks to set VENDOR_SPECIFIC2

[Vinod Koul]

Hello Liam,

On 03-08-20, 20:08, Liam Beguin wrote:
> Hi Vinod,
> 
> On Sat, Jul 18, 2020 at 4:18 PM Liam Beguin <liambeguin@gmail.com> wrote:
> >
> >
> > Hi,
> >
> > I sent another patch[1] to try to address this.
> >
> > The issue seems to come from the fact that the parisc architecture
> > doesn't support cmpxchg on u8 pointers. cmpxchg is called by
> > set_mask_bits.
> >
> 
> This patch has been merged.

Okay great

> 
> > Thanks,
> > Liam
> >
> > [1] https://lore.kernel.org/patchwork/patch/1272617/
> >
> > > ---
> > > 0-DAY CI Kernel Test Service, Intel Corporation
> > > https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
> 
> Did you get a change to look at the initial tusb1210 patch?

Can you please repost after -rc1 is out and rebase on it

Thanks

-- 
~Vinod