Hi, On Wednesday 11 July 2018 05:35 PM, Kunihiko Hayashi wrote: > On Mon, 9 Jul 2018 20:23:19 +0900 <hayashi.kunihiko@xxxxxxxxxxxxx> wrote: > >> Hi Kishon, >> Thank you for your comments. >> >> On Mon, 9 Jul 2018 10:49:50 +0530 <kishon@xxxxxx> 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@xxxxxxxxxxxxx> >>>> Signed-off-by: Motoya Tanigawa <tanigawa.motoya@xxxxxxxxxxxxx> >>>> Signed-off-by: Masami Hiramatsu <masami.hiramatsu@xxxxxxxxxx> >>>> --- >>>> 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@xxxxxxxxxxxxx> >>>> + * Contributors: >>>> + * Motoya Tanigawa <tanigawa.motoya@xxxxxxxxxxxxx> >>>> + * Masami Hiramatsu <masami.hiramatsu@xxxxxxxxxx> >>>> + */ >>>> + >>>> +#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. Thanks Kishon -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html