Re: [PATCH V5 3/7] phy: tegra: xusb: Add USB2 pad power control support for Tegra210

Vinod Koul
2020-08-23T14:03:51Z

On 20-07-20, 15:25, Nagarjuna Kristam wrote:
> Add USB2 pad power on and off API's for TEgra210 and provide its control
> via soc ops. It can be used by operations like charger detect to power on
> and off USB2 pad if needed.
>
> Signed-off-by: Nagarjuna Kristam <nkristam@nvidia.com>
> Acked-by: Thierry Reding <treding@nvidia.com>
> ---
> V5:
> - Made tegra210_usb2_pad_power_on() and tegra210_usb2_pad_power_down() static.
> ---
> V4:
> - No changes
> ---
> V3:
> - Added Acked-by updates to commit message.
> ---
> V2:
> - Patch re-based.
> ---
> drivers/phy/tegra/xusb-tegra210.c | 190 ++++++++++++++++++++++++++------------
> 1 file changed, 133 insertions(+), 57 deletions(-)
>
> diff --git a/drivers/phy/tegra/xusb-tegra210.c b/drivers/phy/tegra/xusb-tegra210.c
> index 66bd461..2e5f71c 100644
> --- a/drivers/phy/tegra/xusb-tegra210.c
> +++ b/drivers/phy/tegra/xusb-tegra210.c
> @@ -994,6 +994,128 @@ static int tegra210_xusb_padctl_id_override(struct tegra_xusb_padctl *padctl,
> return 0;
> }
>
> +static void tegra210_usb2_bias_pad_power_on(struct tegra_xusb_usb2_pad *pad)
> +{
> + struct tegra_xusb_padctl *padctl = pad->base.padctl;
> + u32 value;
> +
> + if (pad->enable++ > 0)
> + return;
> +
> + dev_dbg(padctl->dev, "power on BIAS PAD & USB2 tracking\n");
> +
> + if (clk_prepare_enable(pad->clk))
> + dev_warn(padctl->dev, "failed to enable BIAS PAD & USB2 tracking\n");

do you want to proceed ahead even if clock is not enabled..?

> +
> + value = padctl_readl(padctl, XUSB_PADCTL_USB2_BIAS_PAD_CTL1);
> + value &= ~((XUSB_PADCTL_USB2_BIAS_PAD_CTL1_TRK_START_TIMER_MASK <<
> + XUSB_PADCTL_USB2_BIAS_PAD_CTL1_TRK_START_TIMER_SHIFT) |
> + (XUSB_PADCTL_USB2_BIAS_PAD_CTL1_TRK_DONE_RESET_TIMER_MASK <<
> + XUSB_PADCTL_USB2_BIAS_PAD_CTL1_TRK_DONE_RESET_TIMER_SHIFT));
> + value |= (XUSB_PADCTL_USB2_BIAS_PAD_CTL1_TRK_START_TIMER_VAL <<
> + XUSB_PADCTL_USB2_BIAS_PAD_CTL1_TRK_START_TIMER_SHIFT) |
> + (XUSB_PADCTL_USB2_BIAS_PAD_CTL1_TRK_DONE_RESET_TIMER_VAL <<
> + XUSB_PADCTL_USB2_BIAS_PAD_CTL1_TRK_DONE_RESET_TIMER_SHIFT);

I recently found <linux/bitfield.h> I think this would look lot neater
if we use FIELD_PREP or u32_encode_bits() and you can drop shift defines

Since this is not new code but moved here, I will leave it upto you to
change this, either ways is fine by me.

--
~Vinod