2024-05-07 14:30:59

by André Draszik

[permalink] [raw]
Subject: [PATCH 0/5] a few fixes for the Samsung USB phy driver

Before coming to an agreement on my Samsung USB31 / gs101 phy changes [1]
[2], I decided to split out those changes from that series which can also be
applied independently and add a few additional fixes I had lying around.

This contains mostly cleanup, but also a change to using fsleep() as
recommended by the timers-howto, and a fix for setting the ref frequency for
E850.

These should be less controversial.

Link: https://lore.kernel.org/r/[email protected] [1]
Link: https://lore.kernel.org/r/[email protected] [2]

Signed-off-by: André Draszik <[email protected]>
---
André Draszik (5):
phy: exynos5-usbdrd: uniform order of register bit macros
phy: exynos5-usbdrd: convert udelay() to fsleep()
phy: exynos5-usbdrd: make phy_isol() take a bool for clarity
phy: exynos5-usbdrd: fix definition of EXYNOS5_FSEL_26MHZ
phy: exynos5-usbdrd: set ref clk freq in exynos850_usbdrd_utmi_init()

drivers/phy/samsung/phy-exynos5-usbdrd.c | 95 ++++++++++++++++++--------------
1 file changed, 55 insertions(+), 40 deletions(-)
---
base-commit: 9221b2819b8a4196eecf5476d66201be60fbcf29
change-id: 20240503-samsung-usb-phy-fixes-8b8b6690ffc2

Best regards,
--
André Draszik <[email protected]>



2024-05-07 14:31:19

by André Draszik

[permalink] [raw]
Subject: [PATCH 3/5] phy: exynos5-usbdrd: make phy_isol() take a bool for clarity

on / not on is just a boolean flag and is a bit misleading as currently
on==1 means to turn off the power, and on==0 to turn power on.

Rename the flag and make it a bool to avoid confusion of future readers
of this code. No functional change.

While at it, fix a whitespace issue in nearby comment.

No functional change.

Signed-off-by: André Draszik <[email protected]>
---
drivers/phy/samsung/phy-exynos5-usbdrd.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/phy/samsung/phy-exynos5-usbdrd.c b/drivers/phy/samsung/phy-exynos5-usbdrd.c
index 1b209ab7a268..ed4898741c99 100644
--- a/drivers/phy/samsung/phy-exynos5-usbdrd.c
+++ b/drivers/phy/samsung/phy-exynos5-usbdrd.c
@@ -173,7 +173,7 @@ struct exynos5_usbdrd_phy;

struct exynos5_usbdrd_phy_config {
u32 id;
- void (*phy_isol)(struct phy_usb_instance *inst, u32 on);
+ void (*phy_isol)(struct phy_usb_instance *inst, bool isolate);
void (*phy_init)(struct exynos5_usbdrd_phy *phy_drd);
unsigned int (*set_refclk)(struct phy_usb_instance *inst);
};
@@ -273,14 +273,14 @@ static unsigned int exynos5_rate_to_clk(unsigned long rate, u32 *reg)
}

static void exynos5_usbdrd_phy_isol(struct phy_usb_instance *inst,
- unsigned int on)
+ bool isolate)
{
unsigned int val;

if (!inst->reg_pmu)
return;

- val = on ? 0 : EXYNOS4_PHY_ENABLE;
+ val = isolate ? 0 : EXYNOS4_PHY_ENABLE;

regmap_update_bits(inst->reg_pmu, inst->pmu_offset,
EXYNOS4_PHY_ENABLE, val);
@@ -525,8 +525,8 @@ static int exynos5_usbdrd_phy_power_on(struct phy *phy)
}
}

- /* Power-on PHY*/
- inst->phy_cfg->phy_isol(inst, 0);
+ /* Power-on PHY */
+ inst->phy_cfg->phy_isol(inst, false);

return 0;

@@ -553,7 +553,7 @@ static int exynos5_usbdrd_phy_power_off(struct phy *phy)
dev_dbg(phy_drd->dev, "Request to power_off usbdrd_phy phy\n");

/* Power-off the PHY */
- inst->phy_cfg->phy_isol(inst, 1);
+ inst->phy_cfg->phy_isol(inst, true);

/* Disable VBUS supply */
if (phy_drd->vbus)

--
2.45.0.rc1.225.g2a3ae87e7f-goog


2024-05-07 14:52:51

by André Draszik

[permalink] [raw]
Subject: [PATCH 1/5] phy: exynos5-usbdrd: uniform order of register bit macros

Most of the macros are ordered high -> low, but there are some
outliers.

Order them all uniformly from high to low. This will allow adding
additional register (field) definitions in a consistent way.

While at it, also remove some extra empty lines to group register bit
field definitions together with the relevant register. This makes the
registers easier to distinguish visually.

No functional change.

Signed-off-by: André Draszik <[email protected]>
---
drivers/phy/samsung/phy-exynos5-usbdrd.c | 44 +++++++++++---------------------
1 file changed, 15 insertions(+), 29 deletions(-)

diff --git a/drivers/phy/samsung/phy-exynos5-usbdrd.c b/drivers/phy/samsung/phy-exynos5-usbdrd.c
index 04171eed5b16..2af192c15d78 100644
--- a/drivers/phy/samsung/phy-exynos5-usbdrd.c
+++ b/drivers/phy/samsung/phy-exynos5-usbdrd.c
@@ -35,13 +35,11 @@

/* Exynos5: USB 3.0 DRD PHY registers */
#define EXYNOS5_DRD_LINKSYSTEM 0x04
-
+#define LINKSYSTEM_XHCI_VERSION_CONTROL BIT(27)
#define LINKSYSTEM_FLADJ_MASK (0x3f << 1)
#define LINKSYSTEM_FLADJ(_x) ((_x) << 1)
-#define LINKSYSTEM_XHCI_VERSION_CONTROL BIT(27)

#define EXYNOS5_DRD_PHYUTMI 0x08
-
#define PHYUTMI_OTGDISABLE BIT(6)
#define PHYUTMI_FORCESUSPEND BIT(1)
#define PHYUTMI_FORCESLEEP BIT(0)
@@ -49,40 +47,31 @@
#define EXYNOS5_DRD_PHYPIPE 0x0c

#define EXYNOS5_DRD_PHYCLKRST 0x10
-
#define PHYCLKRST_EN_UTMISUSPEND BIT(31)
-
#define PHYCLKRST_SSC_REFCLKSEL_MASK (0xff << 23)
#define PHYCLKRST_SSC_REFCLKSEL(_x) ((_x) << 23)
-
#define PHYCLKRST_SSC_RANGE_MASK (0x03 << 21)
#define PHYCLKRST_SSC_RANGE(_x) ((_x) << 21)
-
#define PHYCLKRST_SSC_EN BIT(20)
#define PHYCLKRST_REF_SSP_EN BIT(19)
#define PHYCLKRST_REF_CLKDIV2 BIT(18)
-
#define PHYCLKRST_MPLL_MULTIPLIER_MASK (0x7f << 11)
#define PHYCLKRST_MPLL_MULTIPLIER_100MHZ_REF (0x19 << 11)
#define PHYCLKRST_MPLL_MULTIPLIER_50M_REF (0x32 << 11)
#define PHYCLKRST_MPLL_MULTIPLIER_24MHZ_REF (0x68 << 11)
#define PHYCLKRST_MPLL_MULTIPLIER_20MHZ_REF (0x7d << 11)
#define PHYCLKRST_MPLL_MULTIPLIER_19200KHZ_REF (0x02 << 11)
-
-#define PHYCLKRST_FSEL_UTMI_MASK (0x7 << 5)
#define PHYCLKRST_FSEL_PIPE_MASK (0x7 << 8)
+#define PHYCLKRST_FSEL_UTMI_MASK (0x7 << 5)
#define PHYCLKRST_FSEL(_x) ((_x) << 5)
#define PHYCLKRST_FSEL_PAD_100MHZ (0x27 << 5)
#define PHYCLKRST_FSEL_PAD_24MHZ (0x2a << 5)
#define PHYCLKRST_FSEL_PAD_20MHZ (0x31 << 5)
#define PHYCLKRST_FSEL_PAD_19_2MHZ (0x38 << 5)
-
#define PHYCLKRST_RETENABLEN BIT(4)
-
#define PHYCLKRST_REFCLKSEL_MASK (0x03 << 2)
#define PHYCLKRST_REFCLKSEL_PAD_REFCLK (0x2 << 2)
#define PHYCLKRST_REFCLKSEL_EXT_REFCLK (0x3 << 2)
-
#define PHYCLKRST_PORTRESET BIT(1)
#define PHYCLKRST_COMMONONN BIT(0)

@@ -100,30 +89,27 @@
#define PHYREG1_CR_ACK BIT(0)

#define EXYNOS5_DRD_PHYPARAM0 0x1c
-
#define PHYPARAM0_REF_USE_PAD BIT(31)
#define PHYPARAM0_REF_LOSLEVEL_MASK (0x1f << 26)
#define PHYPARAM0_REF_LOSLEVEL (0x9 << 26)

#define EXYNOS5_DRD_PHYPARAM1 0x20
-
#define PHYPARAM1_PCS_TXDEEMPH_MASK (0x1f << 0)
#define PHYPARAM1_PCS_TXDEEMPH (0x1c)

#define EXYNOS5_DRD_PHYTERM 0x24

#define EXYNOS5_DRD_PHYTEST 0x28
-
#define PHYTEST_POWERDOWN_SSP BIT(3)
#define PHYTEST_POWERDOWN_HSP BIT(2)

#define EXYNOS5_DRD_PHYADP 0x2c

#define EXYNOS5_DRD_PHYUTMICLKSEL 0x30
-
#define PHYUTMICLKSEL_UTMI_CLKSEL BIT(2)

#define EXYNOS5_DRD_PHYRESUME 0x34
+
#define EXYNOS5_DRD_LINKPORT 0x44

/* USB 3.0 DRD PHY SS Function Control Reg; accessed by CR_PORT */
@@ -147,28 +133,28 @@

/* Exynos850: USB DRD PHY registers */
#define EXYNOS850_DRD_LINKCTRL 0x04
-#define LINKCTRL_BUS_FILTER_BYPASS(_x) ((_x) << 4)
#define LINKCTRL_FORCE_QACT BIT(8)
+#define LINKCTRL_BUS_FILTER_BYPASS(_x) ((_x) << 4)

#define EXYNOS850_DRD_CLKRST 0x20
-#define CLKRST_LINK_SW_RST BIT(0)
-#define CLKRST_PORT_RST BIT(1)
#define CLKRST_PHY_SW_RST BIT(3)
+#define CLKRST_PORT_RST BIT(1)
+#define CLKRST_LINK_SW_RST BIT(0)

#define EXYNOS850_DRD_UTMI 0x50
-#define UTMI_FORCE_SLEEP BIT(0)
-#define UTMI_FORCE_SUSPEND BIT(1)
-#define UTMI_DM_PULLDOWN BIT(2)
-#define UTMI_DP_PULLDOWN BIT(3)
-#define UTMI_FORCE_BVALID BIT(4)
#define UTMI_FORCE_VBUSVALID BIT(5)
+#define UTMI_FORCE_BVALID BIT(4)
+#define UTMI_DP_PULLDOWN BIT(3)
+#define UTMI_DM_PULLDOWN BIT(2)
+#define UTMI_FORCE_SUSPEND BIT(1)
+#define UTMI_FORCE_SLEEP BIT(0)

#define EXYNOS850_DRD_HSP 0x54
-#define HSP_COMMONONN BIT(8)
-#define HSP_EN_UTMISUSPEND BIT(9)
-#define HSP_VBUSVLDEXT BIT(12)
-#define HSP_VBUSVLDEXTSEL BIT(13)
#define HSP_FSV_OUT_EN BIT(24)
+#define HSP_VBUSVLDEXTSEL BIT(13)
+#define HSP_VBUSVLDEXT BIT(12)
+#define HSP_EN_UTMISUSPEND BIT(9)
+#define HSP_COMMONONN BIT(8)

#define EXYNOS850_DRD_HSP_TEST 0x5c
#define HSP_TEST_SIDDQ BIT(24)

--
2.45.0.rc1.225.g2a3ae87e7f-goog


2024-06-12 08:04:45

by Peter Griffin

[permalink] [raw]
Subject: Re: [PATCH 1/5] phy: exynos5-usbdrd: uniform order of register bit macros

Hi André,

On Tue, 7 May 2024 at 15:14, André Draszik <[email protected]> wrote:
>
> Most of the macros are ordered high -> low, but there are some
> outliers.
>
> Order them all uniformly from high to low. This will allow adding
> additional register (field) definitions in a consistent way.
>
> While at it, also remove some extra empty lines to group register bit
> field definitions together with the relevant register. This makes the
> registers easier to distinguish visually.
>
> No functional change.
>
> Signed-off-by: André Draszik <[email protected]>
> ---

Reviewed-by: Peter Griffin <[email protected]>

regards,

Peter

[..]

2024-06-12 08:47:47

by Peter Griffin

[permalink] [raw]
Subject: Re: [PATCH 3/5] phy: exynos5-usbdrd: make phy_isol() take a bool for clarity

Hi André,

On Tue, 7 May 2024 at 15:14, André Draszik <[email protected]> wrote:
>
> on / not on is just a boolean flag and is a bit misleading as currently
> on==1 means to turn off the power, and on==0 to turn power on.
>
> Rename the flag and make it a bool to avoid confusion of future readers
> of this code. No functional change.
>
> While at it, fix a whitespace issue in nearby comment.
>
> No functional change.
>
> Signed-off-by: André Draszik <[email protected]>
> ---

Reviewed-by: Peter Griffin <[email protected]>

regards,

Peter

[..]

2024-06-12 17:52:28

by Vinod Koul

[permalink] [raw]
Subject: Re: [PATCH 0/5] a few fixes for the Samsung USB phy driver


On Tue, 07 May 2024 15:14:43 +0100, André Draszik wrote:
> Before coming to an agreement on my Samsung USB31 / gs101 phy changes [1]
> [2], I decided to split out those changes from that series which can also be
> applied independently and add a few additional fixes I had lying around.
>
> This contains mostly cleanup, but also a change to using fsleep() as
> recommended by the timers-howto, and a fix for setting the ref frequency for
> E850.
>
> [...]

Applied, thanks!

[1/5] phy: exynos5-usbdrd: uniform order of register bit macros
commit: 2a0dc34bab8ede5fa50378ef206f580303eed8de
[2/5] phy: exynos5-usbdrd: convert udelay() to fsleep()
commit: 27f3d3f6d87f650cc6b3ea08335dea749f1b04aa
[3/5] phy: exynos5-usbdrd: make phy_isol() take a bool for clarity
commit: f2b6fc4d5c9793c556412e9a8ac122670a0d8dcb
[4/5] phy: exynos5-usbdrd: fix definition of EXYNOS5_FSEL_26MHZ
commit: 32b2495e731f2a56118034e9c665e6fe56bbfe3a
[5/5] phy: exynos5-usbdrd: set ref clk freq in exynos850_usbdrd_utmi_init()
commit: d14c14618e851eb25d55807810c2c1791a637712

Best regards,
--
Vinod Koul <[email protected]>


2024-06-12 19:16:25

by Sam Protsenko

[permalink] [raw]
Subject: Re: [PATCH 0/5] a few fixes for the Samsung USB phy driver

On Wed, Jun 12, 2024 at 12:51 PM Vinod Koul <[email protected]> wrote:
>
>
> On Tue, 07 May 2024 15:14:43 +0100, André Draszik wrote:
> > Before coming to an agreement on my Samsung USB31 / gs101 phy changes [1]
> > [2], I decided to split out those changes from that series which can also be
> > applied independently and add a few additional fixes I had lying around.
> >
> > This contains mostly cleanup, but also a change to using fsleep() as
> > recommended by the timers-howto, and a fix for setting the ref frequency for
> > E850.
> >
> > [...]
>
> Applied, thanks!
>
> [1/5] phy: exynos5-usbdrd: uniform order of register bit macros
> commit: 2a0dc34bab8ede5fa50378ef206f580303eed8de
> [2/5] phy: exynos5-usbdrd: convert udelay() to fsleep()
> commit: 27f3d3f6d87f650cc6b3ea08335dea749f1b04aa
> [3/5] phy: exynos5-usbdrd: make phy_isol() take a bool for clarity
> commit: f2b6fc4d5c9793c556412e9a8ac122670a0d8dcb
> [4/5] phy: exynos5-usbdrd: fix definition of EXYNOS5_FSEL_26MHZ
> commit: 32b2495e731f2a56118034e9c665e6fe56bbfe3a
> [5/5] phy: exynos5-usbdrd: set ref clk freq in exynos850_usbdrd_utmi_init()
> commit: d14c14618e851eb25d55807810c2c1791a637712
>

Did somebody actually test it on Exynos850?


> Best regards,
> --
> Vinod Koul <[email protected]>
>