lkml.org 
[lkml]   [2018]   [Jul]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH 2/4] phy: socionext: add USB3 PHY driver for UniPhier SoC
    From
    Date
    Hi,

    On Tuesday 17 July 2018 04:57 PM, Kunihiko Hayashi wrote:
    > Hi Kishon,
    >
    > On Fri, 13 Jul 2018 12:45:06 +0530 <kishon@ti.com> wrote:
    >
    >> Hi,
    >>
    >> On Wednesday 11 July 2018 05:35 PM, Kunihiko Hayashi wrote:
    >>> On Mon, 9 Jul 2018 20:23:19 +0900 <hayashi.kunihiko@socionext.com> wrote:
    >>>
    >>>> Hi Kishon,
    >>>> Thank you for your comments.
    >>>>
    >>>> On Mon, 9 Jul 2018 10:49:50 +0530 <kishon@ti.com> wrote:
    >>>>
    >>>>> Hi,
    >>>>>
    >>>>> On Friday 29 June 2018 02:08 PM, Kunihiko Hayashi wrote:
    >>>>>> Add a driver for PHY interface built into USB3 controller
    >>>>>> implemented in UniPhier SoCs.
    >>>>>> This driver supports High-Speed PHY and Super-Speed PHY.
    >>>>>>
    >>>>>> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
    >>>>>> Signed-off-by: Motoya Tanigawa <tanigawa.motoya@socionext.com>
    >>>>>> Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
    >>>>>> ---
    >>>>>> drivers/phy/Kconfig | 1 +
    >>>>>> drivers/phy/Makefile | 1 +
    >>>>>> drivers/phy/socionext/Kconfig | 12 +
    >>>>>> drivers/phy/socionext/Makefile | 6 +
    >>>>>> drivers/phy/socionext/phy-uniphier-usb3hs.c | 422 ++++++++++++++++++++++++++++
    >>>>>> drivers/phy/socionext/phy-uniphier-usb3ss.c | 369 ++++++++++++++++++++++++
    >>>>>> 6 files changed, 811 insertions(+)
    >>>>>> create mode 100644 drivers/phy/socionext/Kconfig
    >>>>>> create mode 100644 drivers/phy/socionext/Makefile
    >>>>>> create mode 100644 drivers/phy/socionext/phy-uniphier-usb3hs.c
    >>>>>> create mode 100644 drivers/phy/socionext/phy-uniphier-usb3ss.c
    >>>>
    >>>> (snip...)
    >>>>
    >>>>>> --- /dev/null
    >>>>>> +++ b/drivers/phy/socionext/phy-uniphier-usb3hs.c
    >>>>>> @@ -0,0 +1,422 @@
    >>>>>> +// SPDX-License-Identifier: GPL-2.0
    >>>>>> +/*
    >>>>>> + * phy-uniphier-usb3hs.c - HS-PHY driver for Socionext UniPhier USB3 controller
    >>>>>> + * Copyright 2015-2018 Socionext Inc.
    >>>>>> + * Author:
    >>>>>> + * Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
    >>>>>> + * Contributors:
    >>>>>> + * Motoya Tanigawa <tanigawa.motoya@socionext.com>
    >>>>>> + * Masami Hiramatsu <masami.hiramatsu@linaro.org>
    >>>>>> + */
    >>>>>> +
    >>>>>> +#include <linux/bitfield.h>
    >>>>>> +#include <linux/bitops.h>
    >>>>>> +#include <linux/clk.h>
    >>>>>> +#include <linux/io.h>
    >>>>>> +#include <linux/mfd/syscon.h>
    >>>>>> +#include <linux/module.h>
    >>>>>> +#include <linux/nvmem-consumer.h>
    >>>>>> +#include <linux/of.h>
    >>>>>> +#include <linux/of_platform.h>
    >>>>>> +#include <linux/phy/phy.h>
    >>>>>> +#include <linux/platform_device.h>
    >>>>>> +#include <linux/reset.h>
    >>>>>> +#include <linux/slab.h>
    >>>>>> +
    >>>>>> +#define HSPHY_CFG0 0x0
    >>>>>> +#define HSPHY_CFG0_HS_I_MASK GENMASK(31, 28)
    >>>>>> +#define HSPHY_CFG0_HSDISC_MASK GENMASK(27, 26)
    >>>>>> +#define HSPHY_CFG0_SWING_MASK GENMASK(17, 16)
    >>>>>> +#define HSPHY_CFG0_SEL_T_MASK GENMASK(15, 12)
    >>>>>> +#define HSPHY_CFG0_RTERM_MASK GENMASK(7, 6)
    >>>>>> +#define HSPHY_CFG0_TRIMMASK (HSPHY_CFG0_HS_I_MASK \
    >>>>>> + | HSPHY_CFG0_SEL_T_MASK \
    >>>>>> + | HSPHY_CFG0_RTERM_MASK)
    >>>>>> +
    >>>>>> +#define HSPHY_CFG1 0x4
    >>>>>> +#define HSPHY_CFG1_DAT_EN BIT(29)
    >>>>>> +#define HSPHY_CFG1_ADR_EN BIT(28)
    >>>>>> +#define HSPHY_CFG1_ADR_MASK GENMASK(27, 16)
    >>>>>> +#define HSPHY_CFG1_DAT_MASK GENMASK(23, 16)
    >>>>>> +
    >>>>>> +#define MAX_CLKS 3
    >>>>>> +#define MAX_RSTS 2
    >>>>>> +#define MAX_PHY_PARAMS 1
    >>>>>> +
    >>>>>> +struct uniphier_u3hsphy_param {
    >>>>>> + u32 addr;
    >>>>>> + u32 mask;
    >>>>>> + u32 val;
    >>>>>> +};
    >>>>>
    >>>>> I'd like to avoid configure the PHY this way, since it's impossible to know
    >>>>> which register is being configured.
    >>>>
    >>>
    >>> This way might be misunderstood.
    >>> These HS-PHY and SS-PHY have "internal" registers, which are not memory-mapped.
    >>>
    >>> And to access these internal registers, we need to specify the number
    >>> corresponding to the register.
    >>>
    >>> The "addr" in "uniphier_u3hsphy_param" is just the number of the register.
    >>> The "mask" shows a bitfield of the register, that means one of PHY parameters.
    >>> The "value" shows a parameter value to set to the bitfield.
    >>
    >> What does each of these bitfields represent? Which PHY parameter does it
    >> configure. Are they really PHY parameters or they also configure clocks/mux
    >> etc? I would like the configurations to be more descriptive so that we get to
    >> know at least what's getting configured here.
    >
    > These bitfields represent phy parameters only, not include clocks/mux etc.
    > We can express the register (bitfield) name with macros.
    >
    > However, only recommended values are noted for the bitfields in the specsheet,
    > because there are the trimmed values that aren't set as power-on values,
    > and we should use the values for stable operation.

    Calibration values are fine but at least the registers and bitfields has to be
    described using names.

    Thanks
    Kishon

    \
     
     \ /
      Last update: 2018-07-24 06:02    [W:3.960 / U:0.700 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site