Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH v2 4/4] ARM: PWM: add allwinner sun8i pwm support.
From: Hao Zhang @ 2018-05-14 15:39 UTC (permalink / raw)
  To: André Przywara
  Cc: Thierry Reding, Maxime Ripard, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	Mark Rutland, linux-I+IVW8TIWO2tmTQ+vhA3Yw, Chen-Yu Tsai,
	Claudiu Beznea, linux-gpio-u79uwXL29TY76Z2rM5mHXA, open list,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/Allwinner sunXi SoC support,
	linux-pwm-u79uwXL29TY76Z2rM5mHXA,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <8bd4247f-45fd-1385-3ba2-accd7a1e7eb9-5wv7dgnIgG8@public.gmane.org>

2018-02-28 9:55 GMT+08:00 André Przywara <andre.przywara-5wv7dgnIgG8@public.gmane.org>:
> Hi,
>
> On 25/02/18 13:53, hao_zhang wrote:
>> This patch add allwinner sun8i pwm support.
>
> Again, the subject line is too generic. Mention the R40?
>
> Can you elaborate here a bit? Mention that is used on the R40, but not
> other sun8i SoCs, for instance. And mention that this is very different
> from the sun4i-pwm device, so justifies a new driver. Possibly mention
> some features? And that we for now just implement a subset of them.

Thanks for reviews, elaborate it next patch:)

>
>>
>> Signed-off-by: hao_zhang <hao5781286-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>>  drivers/pwm/Kconfig     |  10 ++
>>  drivers/pwm/Makefile    |   1 +
>>  drivers/pwm/pwm-sun8i.c | 401 ++++++++++++++++++++++++++++++++++++++++++++++++
>
> I am not too happy with this name, but I guess there are no better
> alternatives, so it's probably OK to keep it.
>
>>  3 files changed, 412 insertions(+)
>>  create mode 100644 drivers/pwm/pwm-sun8i.c
>>
>> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
>> index 763ee50..7e68d0f 100644
>> --- a/drivers/pwm/Kconfig
>> +++ b/drivers/pwm/Kconfig
>> @@ -444,6 +444,16 @@ config PWM_SUN4I
>>         To compile this driver as a module, choose M here: the module
>>         will be called pwm-sun4i.
>>
>> +config PWM_SUN8I
>> +     tristate "Allwinner PWM SUN8I support"
>> +     depends on ARCH_SUNXI || COMPILE_TEST
>> +     depends on HAS_IOMEM && COMMON_CLK
>> +     help
>> +       Generic PWM framework driver for Allwinner SoCs.
>
> Mmh, not really. So far there is only one SoC using this. Maybe:
>           Driver for the enhanced PWM IP used in some newer Allwinner
>           SoCs.
>
>> +
>> +       To compile this driver as a module, choose M here: the module
>> +       will be called pwm-sun8i.
>> +
>>  config PWM_TEGRA
>>       tristate "NVIDIA Tegra PWM support"
>>       depends on ARCH_TEGRA
>> diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
>> index 0258a74..cd6bf40 100644
>> --- a/drivers/pwm/Makefile
>> +++ b/drivers/pwm/Makefile
>> @@ -44,6 +44,7 @@ obj-$(CONFIG_PWM_STM32)             += pwm-stm32.o
>>  obj-$(CONFIG_PWM_STM32_LP)   += pwm-stm32-lp.o
>>  obj-$(CONFIG_PWM_STMPE)              += pwm-stmpe.o
>>  obj-$(CONFIG_PWM_SUN4I)              += pwm-sun4i.o
>> +obj-$(CONFIG_PWM_SUN8I)              += pwm-sun8i.o
>>  obj-$(CONFIG_PWM_TEGRA)              += pwm-tegra.o
>>  obj-$(CONFIG_PWM_TIECAP)     += pwm-tiecap.o
>>  obj-$(CONFIG_PWM_TIEHRPWM)   += pwm-tiehrpwm.o
>> diff --git a/drivers/pwm/pwm-sun8i.c b/drivers/pwm/pwm-sun8i.c
>> new file mode 100644
>> index 0000000..cf23b0a
>> --- /dev/null
>> +++ b/drivers/pwm/pwm-sun8i.c
>> @@ -0,0 +1,401 @@
>> +#include <linux/bitops.h>
>> +#include <linux/clk.h>
>> +#include <linux/delay.h>
>> +#include <linux/err.h>
>> +#include <linux/io.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/of_device.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/pwm.h>
>> +#include <linux/slab.h>
>> +#include <linux/spinlock.h>
>> +#include <linux/time.h>
>> +#include <linux/regmap.h>
>> +
>> +#define PWM_IRQ_ENABLE_REG   0x0000
>> +#define PCIE(ch)     BIT(ch)
>
> Can you please align those:
> #define PWM_IRQ_ENABLE_REG      0x0000
> #define PCIE(ch)                BIT(ch)
>
> And all those below as well? Which means you might want to insert
> another tab to cater for those longer symbols.

yep, align it is batter :-)

>
>> +
>> +#define PWM_IRQ_STATUS_REG   0x0004
>> +#define PIS(ch)      BIT(ch)
>> +
>> +#define CAPTURE_IRQ_ENABLE_REG       0x0010
>> +#define CFIE(ch)     BIT(ch << 1 + 1)
>> +#define CRIE(ch)     BIT(ch << 1)
>> +
>> +#define CAPTURE_IRQ_STATUS_REG       0x0014
>> +#define CFIS(ch)     BIT(ch << 1 + 1)
>> +#define CRIS(ch)     BIT(ch << 1)
>> +
>> +#define CLK_CFG_REG(ch)      (0x0020 + (ch >> 1) * 4)
>> +#define CLK_SRC      BIT(7)
>> +#define CLK_SRC_BYPASS_SEC   BIT(6)
>> +#define CLK_SRC_BYPASS_FIR   BIT(5)
>> +#define CLK_GATING   BIT(4)
>> +#define CLK_DIV_M    GENMASK(3, 0)
>> +
>> +#define PWM_DZ_CTR_REG(ch)   (0x0030 + (ch >> 1) * 4)
>> +#define PWM_DZ_INTV  GENMASK(15, 8)
>> +#define PWM_DZ_EN    BIT(0)
>> +
>> +#define PWM_ENABLE_REG       0x0040
>> +#define PWM_EN(ch)   BIT(ch)
>> +
>> +#define CAPTURE_ENABLE_REG   0x0044
>> +#define CAP_EN(ch)   BIT(ch)
>> +
>> +#define PWM_CTR_REG(ch)      (0x0060 + ch * 0x20)
>> +#define PWM_PERIOD_RDY       BIT(11)
>> +#define PWM_PUL_START        BIT(10)
>> +#define PWM_MODE     BIT(9)
>> +#define PWM_ACT_STA  BIT(8)
>> +#define PWM_PRESCAL_K        GENMASK(7, 0)
>> +
>> +#define PWM_PERIOD_REG(ch)   (0x0064 + ch * 0x20)
>> +#define PWM_ENTIRE_CYCLE     GENMASK(31, 16)
>> +#define PWM_ACT_CYCLE        GENMASK(15, 0)
>> +
>> +#define PWM_CNT_REG(ch)      (0x0068 + ch * 0x20)
>> +#define PWM_CNT_VAL  GENMASK(15, 0)
>> +
>> +#define CAPTURE_CTR_REG(ch)  (0x006c + ch * 0x20)
>> +#define CAPTURE_CRLF BIT(2)
>> +#define CAPTURE_CFLF BIT(1)
>> +#define CAPINV       BIT(0)
>> +
>> +#define CAPTURE_RISE_REG(ch) (0x0070 + ch * 0x20)
>> +#define CAPTURE_CRLR GENMASK(15, 0)
>> +
>> +#define CAPTURE_FALL_REG(ch) (0x0074 + ch * 0x20)
>> +#define CAPTURE_CFLR GENMASK(15, 0)
>> +
>> +struct sun8i_pwm_data {
>> +     bool has_prescaler_bypass;
>> +     bool has_rdy;
>> +     unsigned int npwm;
>> +};
>
> I believe you don't need this structure. See below.

yep, clock will output directly while bypass has been set,
and equivalent to 50% duty cycles...

>
>> +
>> +struct sun8i_pwm_chip {
>> +     struct pwm_chip chip;
>> +     struct clk *clk;
>> +     void __iomem *base;
>> +     spinlock_t ctrl_lock;
>> +     const struct sun8i_pwm_data *data;
>> +     struct regmap *regmap;
>> +};
>> +
>> +static const u16 div_m_table[] = {
>> +     1,
>> +     2,
>> +     4,
>> +     8,
>> +     16,
>> +     32,
>> +     64,
>> +     128,
>> +     256
>> +};
>
> That looks very much like: "1U << x" to me.

uhmm, i think using table is more explicit and extended...

>
>> +
>> +static inline struct sun8i_pwm_chip *to_sun8i_pwm_chip(struct pwm_chip *chip)
>
> No need for "inline", the compiler knows better. static is enough.

okey :-)

>
>> +{
>> +     return container_of(chip, struct sun8i_pwm_chip, chip);
>> +}
>> +
>> +static u32 sun8i_pwm_read(struct sun8i_pwm_chip *sun8i_pwm,
>> +             unsigned long offset)
>
> Can you please align those continuation lines properly? The first
> character in the new line should be aligned to the first character of
> the first argument. Use tabs first, then fill up with spaces:

Align it next :-)

>
> static u32 sun8i_pwm_read(struct sun8i_pwm_chip *sun8i_pwm,
>                           unsigned long offset)
>
> This applies to the rest of the file as well.
>
>> +{
>> +     u32 val;
>> +
>> +     regmap_read(sun8i_pwm->regmap, offset, &val);
>> +
>> +     return val;
>> +}
>> +
>> +static inline void sun8i_pwm_set_bit(struct sun8i_pwm_chip *sun8i_pwm,
>
> no inline (for those below as well)
>
>> +             unsigned long reg, u32 bit)
>> +{
>> +     regmap_update_bits(sun8i_pwm->regmap, reg, bit, bit);
>> +}
>> +
>> +static inline void sun8i_pwm_clear_bit(struct sun8i_pwm_chip *sun8i_pwm,
>> +             unsigned long reg, u32 bit)
>> +{
>> +     regmap_update_bits(sun8i_pwm->regmap, reg, bit, 0);
>> +}
>> +
>> +static inline void sun8i_pwm_set_value(struct sun8i_pwm_chip *sun8i_pwm,
>> +             unsigned long reg, u32 mask, u32 val)
>> +{
>> +     regmap_update_bits(sun8i_pwm->regmap, reg, mask, val);
>> +}
>> +
>> +static void sun8i_pwm_set_polarity(struct sun8i_pwm_chip *chip, u32 ch,
>> +             enum pwm_polarity polarity)
>> +{
>> +     if (polarity == PWM_POLARITY_NORMAL)
>> +             sun8i_pwm_set_bit(chip, PWM_CTR_REG(ch), PWM_ACT_STA);
>> +     else
>> +             sun8i_pwm_clear_bit(chip, PWM_CTR_REG(ch), PWM_ACT_STA);
>> +}
>> +
>> +static int sun8i_pwm_config(struct sun8i_pwm_chip *sun8i_pwm, u8 ch,
>> +             struct pwm_state *state)
>> +{
>> +     u64 clk_rate, clk_div, val;
>> +     u16 prescaler = 0;
>> +     u8 id = 0;
>> +
>> +     clk_rate = clk_get_rate(sun8i_pwm->clk);
>> +
>> +     if (clk_rate == 24000000)
>> +             sun8i_pwm_clear_bit(sun8i_pwm, CLK_CFG_REG(ch), CLK_SRC);
>> +     else
>> +             sun8i_pwm_set_bit(sun8i_pwm, CLK_CFG_REG(ch), CLK_SRC);
>
> This hardcoded 24MHz looks slightly dodgy and should be replaced with
> some proper code to select the best matching clock, out of a number of
> them given in the DT (see the DT binding mail).
> Without thinking too deeply about it, I guess we try which clocks gives
> the least error for the given configuration. The frequency alone might
> be a good first guide.
> If you can't be bothered with coding this, we might just go ahead with
> the first specified clock and always use this, for now.

Dose the framework support parse 2 or more clk from DT ?
yep, It is better to set the clk automatically

>
>> +
>> +     if (sun8i_pwm->data->has_prescaler_bypass) {
>
> What is this about? I think this is a misunderstanding:
> The bypass bits allows to directly pass on the input clock to the output
> pin, without any actual PWM properties. So if one channel is (by
> chance?) configured for a 50% duty cycle and the same frequency as one
> of the input clocks, you might want to use the bypass bit instead. But I
> don't see many advantages in doing so, so I guess we can ignore it in a
> generic PWM driver.
> Anyway using some hardcoded value from the "data" structure looks just
> wrong to me. I guess you can just remove this, along with the
> has_prescaler_bypass variable from the sun8i_pwm_data structure.

Agree to remove it.

>
>> +             /* pwm output bypass */
>> +             if (ch % 2)
>> +                     sun8i_pwm_set_bit(sun8i_pwm, CLK_CFG_REG(ch),
>> +                                     CLK_SRC_BYPASS_FIR);
>> +             else
>> +                     sun8i_pwm_set_bit(sun8i_pwm, CLK_CFG_REG(ch),
>> +                                     CLK_SRC_BYPASS_SEC);
>> +             return 0;
>> +     }
>> +
>> +     val = state->period * clk_rate;
>> +     do_div(val, NSEC_PER_SEC);
>> +     if (val < 1) {
>> +             dev_err(sun8i_pwm->chip.dev,
>> +                             "Period expects a larger value\n");
>
> Alignment.
> And you might want to hook in here to select a higher frequency input clock.
>
>> +             return -EINVAL;
>> +     }
>> +
>> +     /* calculate and set prescalar, div table, pwn entrie cycle */
>
>                              prescaler             PWM entire
>
> though I believe this "entire cycle" term is an Allwinner invention.
> Wouldn't period be a better term here, also matching the framework?

It seem no...
referent the manual,  "entire cycle" seem means the count of
prescaler_clk(divide by prescaler),
you shoule multiply Tprescaler_clk, then is Tperiod.

>
>> +     clk_div = val;
>> +
>> +     while (clk_div > 65535) {
>> +             prescaler++;
>> +             clk_div = val;
>> +             do_div(clk_div, prescaler + 1);
>> +             do_div(clk_div, div_m_table[id]);
>
>                                 1U << id
>
>> +
>> +             if (prescaler == 255) {
>> +                     prescaler = 0;
>> +                     id++;
>> +                     if (id == 9)
>> +                             return -EINVAL;
>> +             }
>> +     }
>> +
>> +     sun8i_pwm_set_value(sun8i_pwm, PWM_PERIOD_REG(ch),
>> +                     PWM_ENTIRE_CYCLE, clk_div << 16);
>> +     sun8i_pwm_set_value(sun8i_pwm, PWM_CTR_REG(ch),
>> +                     PWM_PRESCAL_K, prescaler << 0);
>> +     sun8i_pwm_set_value(sun8i_pwm, CLK_CFG_REG(ch),
>> +                     CLK_DIV_M, id << 0);
>> +
>> +     /* set duty cycle */
>> +     val = (prescaler + 1) * div_m_table[id] * clk_div;
>
>                                 (1U << id)
>
> You might want to check for the range, though.

Yep :-)

>
>> +     val = state->period;
>> +     do_div(val, clk_div);
>> +     clk_div = state->duty_cycle;
>> +     do_div(clk_div, val);
>> +
>> +     sun8i_pwm_set_value(sun8i_pwm, PWM_PERIOD_REG(ch),
>> +                     PWM_ACT_CYCLE, clk_div << 0);
>> +
>> +     return 0;
>> +}
>> +
>> +static int sun8i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
>> +             struct pwm_state *state)
>> +{
>> +     int ret;
>> +     struct sun8i_pwm_chip *sun8i_pwm = to_sun8i_pwm_chip(chip);
>> +     struct pwm_state cstate;
>> +
>> +     pwm_get_state(pwm, &cstate);
>> +     if (!cstate.enabled) {
>> +             ret = clk_prepare_enable(sun8i_pwm->clk);
>> +             if (ret) {
>> +                     dev_err(chip->dev, "Failed to enable PWM clock\n");
>> +                     return ret;
>> +             }
>> +     }
>> +
>> +     spin_lock(&sun8i_pwm->ctrl_lock);
>> +
>> +     if ((cstate.period != state->period) ||
>> +                     (cstate.duty_cycle != state->duty_cycle)) {
>> +             ret = sun8i_pwm_config(sun8i_pwm, pwm->hwpwm, state);
>> +             if (ret) {
>> +                     spin_unlock(&sun8i_pwm->ctrl_lock);
>> +                     dev_err(chip->dev, "Failed to config PWM\n");
>> +                     return ret;
>> +             }
>> +     }
>> +
>> +     if (state->polarity != cstate.polarity)
>> +             sun8i_pwm_set_polarity(sun8i_pwm, pwm->hwpwm, state->polarity);
>> +
>> +     if (state->enabled) {
>> +             sun8i_pwm_set_bit(sun8i_pwm,
>> +                             CLK_CFG_REG(pwm->hwpwm), CLK_GATING);
>> +
>> +             sun8i_pwm_set_bit(sun8i_pwm,
>> +                             PWM_ENABLE_REG, PWM_EN(pwm->hwpwm));
>> +     } else {
>> +             sun8i_pwm_clear_bit(sun8i_pwm,
>> +                             CLK_CFG_REG(pwm->hwpwm), CLK_GATING);
>> +
>> +             sun8i_pwm_clear_bit(sun8i_pwm,
>> +                             PWM_ENABLE_REG, PWM_EN(pwm->hwpwm));
>> +     }
>> +
>> +     spin_unlock(&sun8i_pwm->ctrl_lock);
>> +
>> +     return 0;
>> +}
>> +
>> +static void sun8i_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
>> +             struct pwm_state *state)
>> +{
>> +     struct sun8i_pwm_chip *sun8i_pwm = to_sun8i_pwm_chip(chip);
>> +     u64 clk_rate, tmp;
>> +     u32 val;
>> +     u16 clk_div, act_cycle;
>> +     u8 prescal, id;
>
> You might want to add a channel variable to increase readability:
>         int channel = pwm->hwpwm;
>

Okey

>> +
>> +     clk_rate = clk_get_rate(sun8i_pwm->clk);
>> +
>> +     val = sun8i_pwm_read(sun8i_pwm, PWM_CTR_REG(pwm->hwpwm));
>> +     if (PWM_ACT_STA & val)
>> +             state->polarity = PWM_POLARITY_NORMAL;
>> +     else
>> +             state->polarity = PWM_POLARITY_INVERSED;
>> +
>> +     prescal = PWM_PRESCAL_K & val;
>> +
>> +     val = sun8i_pwm_read(sun8i_pwm, PWM_ENABLE_REG);
>> +     if (PWM_EN(pwm->hwpwm) & val)
>> +             state->enabled = true;
>> +     else
>> +             state->enabled = false;
>> +
>> +     val = sun8i_pwm_read(sun8i_pwm, PWM_PERIOD_REG(pwm->hwpwm));
>> +     act_cycle = PWM_ACT_CYCLE & val;
>> +     clk_div = val >> 16;
>> +
>> +     val = sun8i_pwm_read(sun8i_pwm, CLK_CFG_REG(pwm->hwpwm));
>> +     id = CLK_DIV_M & val;
>> +
>> +     tmp = act_cycle * prescal * div_m_table[id] * NSEC_PER_SEC;
>> +     state->duty_cycle = DIV_ROUND_CLOSEST_ULL(tmp, clk_rate);
>> +     tmp = clk_div * prescal * div_m_table[id] * NSEC_PER_SEC;
>> +     state->period = DIV_ROUND_CLOSEST_ULL(tmp, clk_rate);
>> +}
>> +
>> +static const struct regmap_config sun8i_pwm_regmap_config = {
>> +     .reg_bits = 32,
>> +     .reg_stride = 4,
>> +     .val_bits = 32,
>> +     .max_register = CAPTURE_FALL_REG(7),
>> +};
>> +
>> +static const struct pwm_ops sun8i_pwm_ops = {
>> +     .apply = sun8i_pwm_apply,
>> +     .get_state = sun8i_pwm_get_state,
>> +     .owner = THIS_MODULE,
>> +};
>> +
>> +static const struct sun8i_pwm_data sun8i_pwm_data_r40 = {
>> +     .has_prescaler_bypass = false,
>
> This is not needed (see my comment above).

yep.

>
>> +     .has_rdy = true,
>
> And this is not used. Copied from sun4i? Where it interestingly isn't
> used either ;-)
>
>> +     .npwm = 8,
>
> I would really love to see this being moved to the DT (see my other mail
> to Thierry about the generic property).
>
> This would mean you don't need a SoC specific structure at all.

okey.

>
>> +};
>> +
>> +static const struct of_device_id sun8i_pwm_dt_ids[] = {
>> +     {
>> +             .compatible = "allwinner,sun8i-r40-pwm",
>> +             .data = &sun8i_pwm_data_r40,
>> +     },
>> +     {},
>> +};
>> +MODULE_DEVICE_TABLE(of, sun8i_pwm_dt_ids);
>> +
>> +static int sun8i_pwm_probe(struct platform_device *pdev)
>> +{
>> +     struct sun8i_pwm_chip *pwm;
>> +     struct resource *res;
>> +     int ret;
>> +     const struct of_device_id *match;
>> +
>> +     match = of_match_device(sun8i_pwm_dt_ids, &pdev->dev);
>> +     if (!match) {
>> +             dev_err(&pdev->dev, "Error: No device match found\n");
>> +             return -ENODEV;
>> +     }
>> +
>> +     pwm = devm_kzalloc(&pdev->dev, sizeof(*pwm), GFP_KERNEL);
>> +     if (!pwm)
>> +             return -ENOMEM;
>> +
>> +     res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +     pwm->base = devm_ioremap_resource(&pdev->dev, res);
>> +     if (IS_ERR(pwm->base))
>> +             return PTR_ERR(pwm->base);
>> +
>> +     pwm->regmap = devm_regmap_init_mmio(&pdev->dev, pwm->base,
>> +                     &sun8i_pwm_regmap_config);
>> +     if (IS_ERR(pwm->regmap)) {
>> +             dev_err(&pdev->dev, "Failed to create regmap\n");
>> +             return PTR_ERR(pwm->regmap);
>> +     }
>> +
>> +     pwm->clk = devm_clk_get(&pdev->dev, NULL);
>> +     if (IS_ERR(pwm->clk))
>> +             return PTR_ERR(pwm->clk);
>
> This would need to be extended to get multiple clocks.

okey.

>
>> +
>> +     pwm->data = match->data;
>> +     pwm->chip.dev = &pdev->dev;
>> +     pwm->chip.ops = &sun8i_pwm_ops;
>> +     pwm->chip.base = -1;
>> +     pwm->chip.npwm = pwm->data->npwm;
>
> It should be fairly easy to initialise this from some DT property.
>
> That's it for the my first review round. Haven't checked the actual
> algorithm and bit assignments yet.
> Did you manage to test this?

Sure :-)
All has been tested on my T3 board (compatible V40, R40)
PWM signal is work well observe from oscilloscope.

>
> Cheers,
> Andre.
>
>> +     pwm->chip.of_xlate = of_pwm_xlate_with_flags;
>> +     pwm->chip.of_pwm_n_cells = 3;
>> +
>> +     spin_lock_init(&pwm->ctrl_lock);
>> +
>> +     ret = pwmchip_add(&pwm->chip);
>> +     if (ret < 0) {
>> +             dev_err(&pdev->dev, "Failed to add PWM chip: %d\n", ret);
>> +             return ret;
>> +     }
>> +
>> +     platform_set_drvdata(pdev, pwm);
>> +
>> +     return 0;
>> +}
>> +
>> +static int sun8i_pwm_remove(struct platform_device *pdev)
>> +{
>> +     struct sun8i_pwm_chip *pwm = platform_get_drvdata(pdev);
>> +
>> +     return pwmchip_remove(&pwm->chip);
>> +}
>> +
>> +static struct platform_driver sun8i_pwm_driver = {
>> +     .driver = {
>> +             .name = "sun8i-pwm",
>> +             .of_match_table = sun8i_pwm_dt_ids,
>> +     },
>> +     .probe = sun8i_pwm_probe,
>> +     .remove = sun8i_pwm_remove,
>> +};
>> +module_platform_driver(sun8i_pwm_driver);
>> +
>> +MODULE_ALIAS("platform: sun8i-pwm");
>> +MODULE_AUTHOR("Hao Zhang <hao5781286-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>");
>> +MODULE_DESCRIPTION("Allwinner sun8i PWM driver");
>> +MODULE_LICENSE("GPL v2");
>>
>

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply

* Re: [PATCH v3] display: panel: Add AUO g070vvn01 display support (800x480)
From: Lukasz Majewski @ 2018-05-14 15:36 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Thierry Reding, David Airlie,
	Rob Herring
  Cc: dri-devel, devicetree, linux-kernel
In-Reply-To: <20180509155913.7562-1-lukma@denx.de>

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

Dear All,

> This commit adds support for AUO's 7.0" display.

If I may gentle remind about this patch.... 

> 
> Signed-off-by: Lukasz Majewski <lukma@denx.de>
> 
> ---
> Changes for v3:
> - Remove not used 'bus-format-override = "rgb565";' property
> 
> Changes for v2:
> - Add *.txt suffix to the auo,g070wn01 file
> ---
>  .../bindings/display/panel/auo,g070vvn01.txt       | 29
> ++++++++++++++++++++
> drivers/gpu/drm/panel/panel-simple.c               | 31
> ++++++++++++++++++++++ 2 files changed, 60 insertions(+) create mode
> 100644
> Documentation/devicetree/bindings/display/panel/auo,g070vvn01.txt
> 
> diff --git
> a/Documentation/devicetree/bindings/display/panel/auo,g070vvn01.txt
> b/Documentation/devicetree/bindings/display/panel/auo,g070vvn01.txt
> new file mode 100644 index 000000000000..49e4105378f6 --- /dev/null
> +++
> b/Documentation/devicetree/bindings/display/panel/auo,g070vvn01.txt
> @@ -0,0 +1,29 @@ +AU Optronics Corporation 7.0" FHD (800 x 480) TFT
> LCD panel +
> +Required properties:
> +- compatible: should be "auo,g070vvn01"
> +- backlight: phandle of the backlight device attached to the panel
> +- power-supply: single regulator to provide the supply voltage
> +
> +Required nodes:
> +- port: Parallel port mapping to connect this display
> +
> +This panel needs single power supply voltage. Its backlight is
> conntrolled +via PWM signal.
> +
> +Example:
> +--------
> +
> +Example device-tree definition when connected to iMX6Q based board
> +
> +	lcd_panel: lcd-panel {
> +		compatible = "auo,g070vvn01";
> +		backlight = <&backlight_lcd>;
> +		power-supply = <&reg_display>;
> +
> +		port {
> +			lcd_panel_in: endpoint {
> +				remote-endpoint = <&lcd_display_out>;
> +			};
> +		};
> +	};
> diff --git a/drivers/gpu/drm/panel/panel-simple.c
> b/drivers/gpu/drm/panel/panel-simple.c index
> cbf1ab404ee7..d9984bdb5bb5 100644 ---
> a/drivers/gpu/drm/panel/panel-simple.c +++
> b/drivers/gpu/drm/panel/panel-simple.c @@ -581,6 +581,34 @@ static
> const struct panel_desc auo_b133htn01 = { },
>  };
>  
> +static const struct display_timing auo_g070vvn01_timings = {
> +	.pixelclock = { 33300000, 34209000, 45000000 },
> +	.hactive = { 800, 800, 800 },
> +	.hfront_porch = { 20, 40, 200 },
> +	.hback_porch = { 87, 40, 1 },
> +	.hsync_len = { 1, 48, 87 },
> +	.vactive = { 480, 480, 480 },
> +	.vfront_porch = { 5, 13, 200 },
> +	.vback_porch = { 31, 31, 29 },
> +	.vsync_len = { 1, 1, 3 },
> +};
> +
> +static const struct panel_desc auo_g070vvn01 = {
> +	.timings = &auo_g070vvn01_timings,
> +	.num_timings = 1,
> +	.bpc = 8,
> +	.size = {
> +		.width = 152,
> +		.height = 91,
> +	},
> +	.delay = {
> +		.prepare = 200,
> +		.enable = 50,
> +		.disable = 50,
> +		.unprepare = 1000,
> +	},
> +};
> +
>  static const struct drm_display_mode auo_g104sn02_mode = {
>  	.clock = 40000,
>  	.hdisplay = 800,
> @@ -2095,6 +2123,9 @@ static const struct of_device_id
> platform_of_match[] = { .compatible = "auo,b133xtn01",
>  		.data = &auo_b133xtn01,
>  	}, {
> +		.compatible = "auo,g070vvn01",
> +		.data = &auo_g070vvn01,
> +	}, {
>  		.compatible = "auo,g104sn02",
>  		.data = &auo_g104sn02,
>  	}, {




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* Re: [PATCH 2/6] pinctrl: sh-pfc: Initial R8A77990 PFC support
From: Geert Uytterhoeven @ 2018-05-14 15:32 UTC (permalink / raw)
  To: Yoshihiro Shimoda
  Cc: Laurent Pinchart, Geert Uytterhoeven, Linus Walleij, Rob Herring,
	Mark Rutland, Linux-Renesas, open list:GPIO SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Takeshi Kihara
In-Reply-To: <1526008947-26667-3-git-send-email-yoshihiro.shimoda.uh@renesas.com>

Hi Shimoda-san,

On Fri, May 11, 2018 at 5:22 AM, Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> From: Takeshi Kihara <takeshi.kihara.df@renesas.com>
>
> This patch adds initial pinctrl driver to support for the R8A77990 SoC.
>
> Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
i.e. will queue in sh-pfc-for-v4.18...

Note that I only looked at the CPU_ALL_PORT() macro, and the various
register addresses.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: [PATCH 1/6] pinctrl: sh-pfc: Add PORT_GP_11 helper macro
From: Geert Uytterhoeven @ 2018-05-14 15:23 UTC (permalink / raw)
  To: Yoshihiro Shimoda
  Cc: Laurent Pinchart, Geert Uytterhoeven, Linus Walleij, Rob Herring,
	Mark Rutland, Linux-Renesas, open list:GPIO SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Takeshi Kihara
In-Reply-To: <1526008947-26667-2-git-send-email-yoshihiro.shimoda.uh@renesas.com>

Hi Shimoda-san,

On Fri, May 11, 2018 at 5:22 AM, Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> From: Takeshi Kihara <takeshi.kihara.df@renesas.com>
>
> This follows the style of existion PORT_GP_X macros and
> will be used by a follow-up patch for the r8a77990 SoC.

Thanks for your patch!

> Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
i.e. will queue in sh-pfc-for-v4.18...

> --- a/drivers/pinctrl/sh-pfc/sh_pfc.h
> +++ b/drivers/pinctrl/sh-pfc/sh_pfc.h
> @@ -415,9 +415,13 @@ struct sh_pfc_soc_info {
>         PORT_GP_CFG_1(bank, 9,  fn, sfx, cfg)
>  #define PORT_GP_10(bank, fn, sfx)      PORT_GP_CFG_10(bank, fn, sfx, 0)
>
> -#define PORT_GP_CFG_12(bank, fn, sfx, cfg)                             \
> +#define PORT_GP_CFG_11(bank, fn, sfx, cfg)                             \
>         PORT_GP_CFG_10(bank, fn, sfx, cfg),                             \
> -       PORT_GP_CFG_1(bank, 10, fn, sfx, cfg),                          \
> +       PORT_GP_CFG_1(bank, 10,  fn, sfx, cfg)

... with one space in fron of "fn" removed.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: [PATCH v3 4/4] drm/rockchip: support dp training outside dp firmware
From: Sean Paul @ 2018-05-14 15:18 UTC (permalink / raw)
  To: Lin Huang
  Cc: devicetree, airlied, briannorris, dianders, linux-kernel,
	linux-rockchip, robh+dt, dri-devel, zyw, daniel.vetter,
	linux-arm-kernel
In-Reply-To: <1526291635-31122-4-git-send-email-hl@rock-chips.com>

On Mon, May 14, 2018 at 05:53:55PM +0800, Lin Huang wrote:
> DP firmware uses fixed phy config values to do training, but some
> boards need to adjust these values to fit for their unique hardware
> design. So if the phy is using custom config values, do software
> link training instead of relying on firmware, if software training
> fail, keep firmware training as a fallback if sw training fails.
> 
> Signed-off-by: Chris Zhong <zyw@rock-chips.com>
> Signed-off-by: Lin Huang <hl@rock-chips.com>
> ---
> Changes in v2:
> - update patch following Enric suggest
> Changes in v3:
> - use variable fw_training instead sw_training_success
> - base on DP SPCE, if training fail use lower link rate to retry training
> 
>  drivers/gpu/drm/rockchip/Makefile               |   3 +-
>  drivers/gpu/drm/rockchip/cdn-dp-core.c          |  24 +-
>  drivers/gpu/drm/rockchip/cdn-dp-core.h          |   2 +
>  drivers/gpu/drm/rockchip/cdn-dp-link-training.c | 416 ++++++++++++++++++++++++
>  drivers/gpu/drm/rockchip/cdn-dp-reg.c           |  31 +-
>  drivers/gpu/drm/rockchip/cdn-dp-reg.h           |  38 ++-
>  6 files changed, 501 insertions(+), 13 deletions(-)
>  create mode 100644 drivers/gpu/drm/rockchip/cdn-dp-link-training.c
> 
> diff --git a/drivers/gpu/drm/rockchip/Makefile b/drivers/gpu/drm/rockchip/Makefile
> index a314e21..b932f62 100644
> --- a/drivers/gpu/drm/rockchip/Makefile
> +++ b/drivers/gpu/drm/rockchip/Makefile
> @@ -9,7 +9,8 @@ rockchipdrm-y := rockchip_drm_drv.o rockchip_drm_fb.o \
>  rockchipdrm-$(CONFIG_DRM_FBDEV_EMULATION) += rockchip_drm_fbdev.o
>  
>  rockchipdrm-$(CONFIG_ROCKCHIP_ANALOGIX_DP) += analogix_dp-rockchip.o
> -rockchipdrm-$(CONFIG_ROCKCHIP_CDN_DP) += cdn-dp-core.o cdn-dp-reg.o
> +rockchipdrm-$(CONFIG_ROCKCHIP_CDN_DP) += cdn-dp-core.o cdn-dp-reg.o \
> +					cdn-dp-link-training.o
>  rockchipdrm-$(CONFIG_ROCKCHIP_DW_HDMI) += dw_hdmi-rockchip.o
>  rockchipdrm-$(CONFIG_ROCKCHIP_DW_MIPI_DSI) += dw-mipi-dsi.o
>  rockchipdrm-$(CONFIG_ROCKCHIP_INNO_HDMI) += inno_hdmi.o
> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c b/drivers/gpu/drm/rockchip/cdn-dp-core.c
> index cce64c1..d9d0d4d 100644
> --- a/drivers/gpu/drm/rockchip/cdn-dp-core.c
> +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c
> @@ -629,11 +629,13 @@ static void cdn_dp_encoder_enable(struct drm_encoder *encoder)
>  			goto out;
>  		}
>  	}
> -
> -	ret = cdn_dp_set_video_status(dp, CONTROL_VIDEO_IDLE);
> -	if (ret) {
> -		DRM_DEV_ERROR(dp->dev, "Failed to idle video %d\n", ret);
> -		goto out;
> +	if (dp->use_fw_training == true) {
> +		ret = cdn_dp_set_video_status(dp, CONTROL_VIDEO_IDLE);
> +		if (ret) {
> +			DRM_DEV_ERROR(dp->dev,
> +				      "Failed to idle video %d\n", ret);
> +			goto out;
> +		}
>  	}
>  
>  	ret = cdn_dp_config_video(dp);
> @@ -642,11 +644,15 @@ static void cdn_dp_encoder_enable(struct drm_encoder *encoder)
>  		goto out;
>  	}
>  
> -	ret = cdn_dp_set_video_status(dp, CONTROL_VIDEO_VALID);
> -	if (ret) {
> -		DRM_DEV_ERROR(dp->dev, "Failed to valid video %d\n", ret);
> -		goto out;
> +	if (dp->use_fw_training == true) {
> +		ret = cdn_dp_set_video_status(dp, CONTROL_VIDEO_VALID);
> +		if (ret) {
> +			DRM_DEV_ERROR(dp->dev,
> +				"Failed to valid video %d\n", ret);
> +			goto out;
> +		}
>  	}
> +
>  out:
>  	mutex_unlock(&dp->lock);
>  }
> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.h b/drivers/gpu/drm/rockchip/cdn-dp-core.h
> index 46159b2..77a9793 100644
> --- a/drivers/gpu/drm/rockchip/cdn-dp-core.h
> +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.h
> @@ -84,6 +84,7 @@ struct cdn_dp_device {
>  	bool connected;
>  	bool active;
>  	bool suspended;
> +	bool use_fw_training;
>  
>  	const struct firmware *fw;	/* cdn dp firmware */
>  	unsigned int fw_version;	/* cdn fw version */
> @@ -106,6 +107,7 @@ struct cdn_dp_device {
>  	u8 ports;
>  	u8 lanes;
>  	int active_port;
> +	u8 train_set[4];
>  
>  	u8 dpcd[DP_RECEIVER_CAP_SIZE];
>  	bool sink_has_audio;
> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-link-training.c b/drivers/gpu/drm/rockchip/cdn-dp-link-training.c
> new file mode 100644
> index 0000000..b8fd5bc
> --- /dev/null
> +++ b/drivers/gpu/drm/rockchip/cdn-dp-link-training.c

/snip

> +static int cdn_dp_get_lower_link_rate(struct cdn_dp_device *dp)
> +{
> +	if (dp->link.rate == DP_LINK_BW_1_62)
> +		return -EINVAL;
> +	else if (dp->link.rate == DP_LINK_BW_2_7)
> +			dp->link.rate = DP_LINK_BW_1_62;

Extra indent

> +	else
> +		dp->link.rate = DP_LINK_BW_2_7;


This is better expressed as a switch statement:


        switch (dp->link.rate) {
        case DP_LINK_BW_1_62:
                return -EINVAL;
        case DP_LINK_BW_2_7:
                dp->link.rate = DP_LINK_BW_1_62:
                break;
        default:
                dp->link.rate = DP_LINK_BW_2_7:
                break;
        }

You might also consider adding an additional case since there are rates higher
than 5.4GHz. ie:
        case DP_LINK_BW_5_4:
                dp->link.rate = DP_LINK_BW_2_7:
                break;
        default:
                dp->link.rate = DP_LINK_BW_5_4:
                break;


> +
> +	return 0;
> +}
> +
> +int cdn_dp_software_train_link(struct cdn_dp_device *dp)
> +{
> +	int ret, stop_err;
> +	u8 link_config[2];
> +	u32 rate, sink_max, source_max;
> +
> +	ret = drm_dp_dpcd_read(&dp->aux, DP_DPCD_REV, dp->dpcd,
> +			       sizeof(dp->dpcd));
> +	if (ret < 0) {
> +		DRM_DEV_ERROR(dp->dev, "Failed to get caps %d\n", ret);
> +		return ret;
> +	}
> +
> +	source_max = dp->lanes;
> +	sink_max = drm_dp_max_lane_count(dp->dpcd);
> +	dp->link.num_lanes = min(source_max, sink_max);
> +
> +	source_max = drm_dp_bw_code_to_link_rate(CDN_DP_MAX_LINK_RATE);
> +	sink_max = drm_dp_max_link_rate(dp->dpcd);
> +	rate = min(source_max, sink_max);
> +	dp->link.rate = drm_dp_link_rate_to_bw_code(rate);
> +
> +retry:
> +	/* Write the link configuration data */
> +	link_config[0] = dp->link.rate;
> +	link_config[1] = dp->link.num_lanes;
> +	if (drm_dp_enhanced_frame_cap(dp->dpcd))
> +		link_config[1] |= DP_LANE_COUNT_ENHANCED_FRAME_EN;
> +	drm_dp_dpcd_write(&dp->aux, DP_LINK_BW_SET, link_config, 2);
> +
> +	link_config[0] = 0;
> +	link_config[1] = 0;
> +	if (dp->dpcd[DP_MAIN_LINK_CHANNEL_CODING] & 0x01)
> +		link_config[1] = DP_SET_ANSI_8B10B;
> +	drm_dp_dpcd_write(&dp->aux, DP_DOWNSPREAD_CTRL, link_config, 2);
> +
> +	ret = cdn_dp_link_training_clock_recovery(dp);
> +	if (ret) {
> +		if (cdn_dp_get_lower_link_rate(dp)) {
> +			DRM_ERROR("training clock recovery fail, err: %d\n",
> +				  ret);
> +			goto stop_training;
> +		}
> +
> +		/* use lower link rate to retraining */
> +		goto retry;
> +	}
> +
> +	ret = cdn_dp_link_training_channel_equalization(dp);
> +	if (ret) {
> +		if (cdn_dp_get_lower_link_rate(dp)) {
> +			DRM_ERROR("training channel equalization fail, err: %d\n",
> +				  ret);
> +			goto stop_training;
> +		}
> +
> +		/* use lower link rate to retraining */
> +		goto retry;
> +	}
> +
> +stop_training:
> +	stop_err = cdn_dp_stop_link_train(dp);
> +	if (stop_err) {
> +		DRM_ERROR("stop training fail, error: %d\n", stop_err);
> +		return stop_err;
> +	}
> +
> +	return ret;

Using labels to do loops reduces readability, it seems like you can also pull
out the downspread control write.

        link_config[0] = 0;
        link_config[1] = 0;
        if (dp->dpcd[DP_MAIN_LINK_CHANNEL_CODING] & 0x01)
                link_config[1] = DP_SET_ANSI_8B10B;
        drm_dp_dpcd_write(&dp->aux, DP_DOWNSPREAD_CTRL, link_config, 2);

        while (true) {
                /* Write the link configuration data */
                link_config[0] = dp->link.rate;
                link_config[1] = dp->link.num_lanes;
                if (drm_dp_enhanced_frame_cap(dp->dpcd))
                        link_config[1] |= DP_LANE_COUNT_ENHANCED_FRAME_EN;
                drm_dp_dpcd_write(&dp->aux, DP_LINK_BW_SET, link_config, 2);

                ret = cdn_dp_link_training_clock_recovery(dp);
                if (ret) {
                        if (!cdn_dp_get_lower_link_rate(dp))
                                continue;

                        DRM_ERROR("training clock recovery failed: %d\n", ret);
                        break;
                }

                ret = cdn_dp_link_training_channel_equalization(dp);
                if (ret) {
                        if (!cdn_dp_get_lower_link_rate(dp))
                                continue;

                        DRM_ERROR("training channel eq failed: %d\n", ret);
                        break;
                }

                return 0;
        }

        stop_err = cdn_dp_stop_link_train(dp);
        if (stop_err) {
                DRM_ERROR("stop training fail, error: %d\n", stop_err);
                return stop_err;
        }

        return ret;

> +
> +stop_training:
> +	stop_err = cdn_dp_stop_link_train(dp);
> +	if (stop_err) {
> +		DRM_ERROR("stop training fail, error: %d\n", stop_err);
> +		return stop_err;
> +	}
> +
> +	return ret;

> +}
> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-reg.c b/drivers/gpu/drm/rockchip/cdn-dp-reg.c
> index 979355d..e1273e6 100644
> --- a/drivers/gpu/drm/rockchip/cdn-dp-reg.c
> +++ b/drivers/gpu/drm/rockchip/cdn-dp-reg.c
> @@ -17,7 +17,9 @@
>  #include <linux/delay.h>
>  #include <linux/io.h>
>  #include <linux/iopoll.h>
> +#include <linux/phy/phy.h>
>  #include <linux/reset.h>
> +#include <soc/rockchip/rockchip_phy_typec.h>
>  
>  #include "cdn-dp-core.h"
>  #include "cdn-dp-reg.h"
> @@ -189,7 +191,7 @@ static int cdn_dp_mailbox_send(struct cdn_dp_device *dp, u8 module_id,
>  	return 0;
>  }
>  
> -static int cdn_dp_reg_write(struct cdn_dp_device *dp, u16 addr, u32 val)
> +int cdn_dp_reg_write(struct cdn_dp_device *dp, u16 addr, u32 val)
>  {
>  	u8 msg[6];
>  
> @@ -609,6 +611,31 @@ int cdn_dp_train_link(struct cdn_dp_device *dp)
>  {
>  	int ret;
>  
> +	/*
> +	 * DP firmware uses fixed phy config values to do training, but some
> +	 * boards need to adjust these values to fit for their unique hardware
> +	 * design. So if the phy is using custom config values, do software
> +	 * link training instead of relying on firmware, if software training

This comment is no longer accurate.

> +	 * fail, keep firmware training as a fallback if sw training fails.
> +	 */
> +	ret = cdn_dp_software_train_link(dp);
> +	if (ret) {
> +		DRM_DEV_ERROR(dp->dev,
> +			"Failed to do software training %d\n", ret);
> +		goto do_fw_training;
> +	}
> +	ret = cdn_dp_reg_write(dp, SOURCE_HDTX_CAR, 0xf);
> +	if (ret) {
> +		DRM_DEV_ERROR(dp->dev,
> +		"Failed to write SOURCE_HDTX_CAR register %d\n", ret);
> +		goto do_fw_training;
> +	}
> +	dp->use_fw_training = false;
> +	return 0;
> +
> +do_fw_training:
> +	dp->use_fw_training = true;
> +	DRM_DEV_DEBUG_KMS(dp->dev, "use fw training\n");
>  	ret = cdn_dp_training_start(dp);
>  	if (ret) {
>  		DRM_DEV_ERROR(dp->dev, "Failed to start training %d\n", ret);
> @@ -623,7 +650,7 @@ int cdn_dp_train_link(struct cdn_dp_device *dp)
>  
>  	DRM_DEV_DEBUG_KMS(dp->dev, "rate:0x%x, lanes:%d\n", dp->link.rate,
>  			  dp->link.num_lanes);
> -	return ret;
> +	return 0;
>  }
>  
>  int cdn_dp_set_video_status(struct cdn_dp_device *dp, int active)
> 

/snip

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply

* Re: [PATCH/RFC v3 1/4] base: devcon: add a new API to find the graph
From: Randy Dunlap @ 2018-05-14 15:09 UTC (permalink / raw)
  To: Yoshihiro Shimoda, gregkh, robh+dt, mark.rutland
  Cc: heikki.krogerus, hdegoede, andy.shevchenko, linux-usb,
	linux-renesas-soc, devicetree
In-Reply-To: <1526289360-3997-2-git-send-email-yoshihiro.shimoda.uh@renesas.com>

On 05/14/2018 02:15 AM, Yoshihiro Shimoda wrote:
> This patch adds a new API "device_connection_find_by_graph()" to
> find device connection by using graph.
> 
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> ---
>  Documentation/driver-api/device_connection.rst |  4 +--
>  drivers/base/devcon.c                          | 43 ++++++++++++++++++++++++++
>  include/linux/device.h                         |  2 ++
>  3 files changed, 47 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/driver-api/device_connection.rst b/Documentation/driver-api/device_connection.rst
> index affbc556..2e2d26f 100644
> --- a/Documentation/driver-api/device_connection.rst
> +++ b/Documentation/driver-api/device_connection.rst
> @@ -19,7 +19,7 @@ Device connections alone do not create a dependency between the two devices.
>  They are only descriptions which are not tied to either of the devices directly.
>  A dependency between the two devices exists only if one of the two endpoint
>  devices requests a reference to the other. The descriptions themselves can be
> -defined in firmware (not yet supported) or they can be built-in.
> +defined in firmware or they can be built-in.
>  
>  Usage
>  -----
> @@ -40,4 +40,4 @@ API
>  ---
>  
>  .. kernel-doc:: drivers/base/devcon.c
> -   : functions: device_connection_find_match device_connection_find device_connection_add device_connection_remove
> +   : functions: device_connection_find_match device_connection_find device_connection_add device_connection_remove device_connection_find_by_graph

BTW, that line above should be like:
      :functions: ...
i.e., no space after the first ':'.

I have sent a patch for that and Jon Corbet has applied it to his docs tree.

-- 
~Randy

^ permalink raw reply

* Re: [PATCH v3 3/4] phy: rockchip-typec: support variable phy config value
From: Sean Paul @ 2018-05-14 14:58 UTC (permalink / raw)
  To: Lin Huang
  Cc: devicetree, airlied, briannorris, dianders, linux-kernel,
	linux-rockchip, robh+dt, dri-devel, zyw, daniel.vetter,
	linux-arm-kernel
In-Reply-To: <1526291635-31122-3-git-send-email-hl@rock-chips.com>

On Mon, May 14, 2018 at 05:53:54PM +0800, Lin Huang wrote:
> the phy config values used to fix in dp firmware, but some boards
> need change these values to do training and get the better eye diagram
> result. So support that in phy driver.
> 
> Signed-off-by: Chris Zhong <zyw@rock-chips.com>
> Signed-off-by: Lin Huang <hl@rock-chips.com>
> ---
> Changes in v2:
> - update patch following Enric suggest
> Changes in v3:
> - delete need_software_training variable
> - add default phy config value, if dts do not define phy config value, use these value
> 
>  drivers/phy/rockchip/phy-rockchip-typec.c | 305 ++++++++++++++++++++----------
>  include/soc/rockchip/rockchip_phy_typec.h |  63 ++++++
>  2 files changed, 270 insertions(+), 98 deletions(-)
>  create mode 100644 include/soc/rockchip/rockchip_phy_typec.h
> 
> diff --git a/drivers/phy/rockchip/phy-rockchip-typec.c b/drivers/phy/rockchip/phy-rockchip-typec.c
> index 76a4b58..10253ad 100644
> --- a/drivers/phy/rockchip/phy-rockchip-typec.c
> +++ b/drivers/phy/rockchip/phy-rockchip-typec.c

/snip

>  
> +/* default phy config */
> +struct phy_config configs[3][4] = {

static const

Also, configs isn't a good name. How about tcphy_default_config?


> +	{{ 0x2a, 0x00 },

Can you please expand the assignment for all of these, ie:

         { .swing = 0x2a, .pe = 0x00 },

> +	 { 0x1f, 0x15 },
> +	 { 0x14, 0x22 },
> +	 { 0x02, 0x2b } },
> +
> +	{{ 0x21, 0x00 },
> +	 { 0x12, 0x15 },
> +	 { 0x02, 0x22 },
> +	 {    0,    0 } },
> +
> +	{{ 0x15, 0x00 },
> +	 { 0x00, 0x15 },
> +	 {    0,    0 },
> +	 {    0,    0 } },
> +};
> +

/snip

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply

* Re: [PATCH] thermal: qcom: tsens: Allow number of sensors to come from DT
From: Amit Kucheria @ 2018-05-14 14:45 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Zhang Rui, Eduardo Valentin, Rob Herring, Mark Rutland,
	Rajendra Nayak, Linux PM list, devicetree, LKML, linux-arm-msm
In-Reply-To: <20180507235339.8836-1-bjorn.andersson@linaro.org>

On Tue, May 8, 2018 at 2:53 AM, Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
> For platforms that has multiple copies of the TSENS hardware block it's
> necessary to be able to specify the number of sensors per block in DeviceTree.

I assume you want to replace the hardcoded num_sensors values in the
tsens-xxxx.c files with values in device tree atleast on platforms
that use devicetree?

> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
>  .../devicetree/bindings/thermal/qcom-tsens.txt       |  1 +
>  drivers/thermal/qcom/tsens.c                         | 12 +++++++++---
>  2 files changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.txt b/Documentation/devicetree/bindings/thermal/qcom-tsens.txt
> index 292ed89d900b..06195e8f35e2 100644
> --- a/Documentation/devicetree/bindings/thermal/qcom-tsens.txt
> +++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.txt
> @@ -8,6 +8,7 @@ Required properties:
>
>  - reg: Address range of the thermal registers
>  - #thermal-sensor-cells : Should be 1. See ./thermal.txt for a description.
> +- #qcom,sensors: Number of sensors in tsens block
>  - Refer to Documentation/devicetree/bindings/nvmem/nvmem.txt to know how to specify
>  nvmem cells
>
> diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
> index 3f9fe6aa51cc..20f3b87d7667 100644
> --- a/drivers/thermal/qcom/tsens.c
> +++ b/drivers/thermal/qcom/tsens.c
> @@ -116,6 +116,7 @@ static int tsens_probe(struct platform_device *pdev)
>         struct tsens_device *tmdev;
>         const struct tsens_data *data;
>         const struct of_device_id *id;
> +       u32 num_sensors;
>
>         if (pdev->dev.of_node)
>                 dev = &pdev->dev;
> @@ -130,18 +131,23 @@ static int tsens_probe(struct platform_device *pdev)
>         else
>                 data = &data_8960;
>
> -       if (data->num_sensors <= 0) {
> +       num_sensors = data->num_sensors;
> +
> +       if (np)
> +               of_property_read_u32(np, "#qcom,sensors", &num_sensors);
> +
> +       if (num_sensors <= 0) {
>                 dev_err(dev, "invalid number of sensors\n");
>                 return -EINVAL;
>         }
>
>         tmdev = devm_kzalloc(dev, sizeof(*tmdev) +
> -                            data->num_sensors * sizeof(*s), GFP_KERNEL);
> +                            num_sensors * sizeof(*s), GFP_KERNEL);
>         if (!tmdev)
>                 return -ENOMEM;
>
>         tmdev->dev = dev;
> -       tmdev->num_sensors = data->num_sensors;
> +       tmdev->num_sensors = num_sensors;
>         tmdev->ops = data->ops;
>         for (i = 0;  i < tmdev->num_sensors; i++) {
>                 if (data->hw_ids)
> --
> 2.17.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v2 4/4] ARM: PWM: add allwinner sun8i pwm support.
From: Hao Zhang @ 2018-05-14 14:45 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Thierry Reding, robh+dt-DgEjT+Ai2ygdnm+yROfE0A, Mark Rutland,
	linux-I+IVW8TIWO2tmTQ+vhA3Yw, Chen-Yu Tsai, Claudiu Beznea,
	linux-gpio-u79uwXL29TY76Z2rM5mHXA, open list,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/Allwinner sunXi SoC support,
	linux-pwm-u79uwXL29TY76Z2rM5mHXA,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <20180226090038.etk5q4pd4rl5dvf6-ZC1Zs529Oq4@public.gmane.org>

2018-02-26 17:00 GMT+08:00 Maxime Ripard <maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org>:
> Hi,
>
> Thanks for respinning this serie. It looks mostly good, but you still
> have a quite significant number of checkpatch (--strict) warnings that
> you should address.

Thanks for reviews :) ,i'm sorry for that, it will be fixed next time.
and, besides, in what situation were the checkpatch warning can be ignore?

>
> On Sun, Feb 25, 2018 at 09:53:08PM +0800, hao_zhang wrote:
>> +#define CAPTURE_IRQ_ENABLE_REG       0x0010
>> +#define CFIE(ch)     BIT(ch << 1 + 1)
>> +#define CRIE(ch)     BIT(ch << 1)
>
> You should also put your argument between parentheses here (and in all
> your other macros).

Do you mean like this ?
#define CFIE(ch)     BIT((ch) << 1 + 1)
#define CRIE(ch)     BIT((ch) << 1)

>
>> +static const u16 div_m_table[] = {
>> +     1,
>> +     2,
>> +     4,
>> +     8,
>> +     16,
>> +     32,
>> +     64,
>> +     128,
>> +     256
>> +};
>
> If this is just a power of two, you can use either the power of two /
> ilog2 to switch back and forth, instead of using that table.

I think using table is more explicit and extended...

>
>> +static int sun8i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
>> +             struct pwm_state *state)
>> +{
>> +     int ret;
>> +     struct sun8i_pwm_chip *sun8i_pwm = to_sun8i_pwm_chip(chip);
>> +     struct pwm_state cstate;
>> +
>> +     pwm_get_state(pwm, &cstate);
>> +     if (!cstate.enabled) {
>> +             ret = clk_prepare_enable(sun8i_pwm->clk);
>> +             if (ret) {
>> +                     dev_err(chip->dev, "Failed to enable PWM clock\n");
>> +                     return ret;
>> +             }
>> +     }
>> +
>> +     spin_lock(&sun8i_pwm->ctrl_lock);
>
> What do you need that spinlock for? Can you use a mutex instead?
It should be remove.
>
> Thanks!
> Maxime
>
> --
> Maxime Ripard, Bootlin (formerly Free Electrons)
> Embedded Linux and Kernel engineering
> https://bootlin.com

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply

* Re: [PATCH v2 05/27] dts: bindings: Document device tree binding for CATU
From: Mathieu Poirier @ 2018-05-14 14:42 UTC (permalink / raw)
  To: Rob Herring
  Cc: Suzuki K Poulose, linux-arm-kernel, linux-kernel@vger.kernel.org,
	Mike Leach, Robert Walker, Mark Rutland, Will Deacon,
	Robin Murphy, Sudeep Holla, Frank Rowand, John Horley, devicetree,
	Mathieu Poirier
In-Reply-To: <CAL_JsqJsZDPHxWA5tax4_ecBtR7Yvhi+DzxQkyh4bpnu1Za0yg@mail.gmail.com>

On Fri, May 11, 2018 at 11:05:56AM -0500, Rob Herring wrote:
> On Tue, May 8, 2018 at 10:40 AM, Suzuki K Poulose
> <Suzuki.Poulose@arm.com> wrote:
> >
> >
> > Rob, Mathieu,
> >
> >
> > On 03/05/18 18:42, Mathieu Poirier wrote:
> >>
> >> On 1 May 2018 at 07:10, Rob Herring <robh@kernel.org> wrote:
> >>>
> >>> On Tue, May 01, 2018 at 10:10:35AM +0100, Suzuki K Poulose wrote:
> >>>>
> >>>> Document CATU device-tree bindings. CATU augments the TMC-ETR
> >>>> by providing an improved Scatter Gather mechanism for streaming
> >>>> trace data to non-contiguous system RAM pages.
> >>>>
> >>>> Cc: devicetree@vger.kernel.org
> >>>> Cc: frowand.list@gmail.com
> >>>> Cc: Rob Herring <robh@kernel.org>
> >>>> Cc: Mark Rutland <mark.rutland@arm.com>
> >>>> Cc: Mathieu Poirier <mathieu.poirier@arm.com>
> >>>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> >>>> ---
> >>>>   .../devicetree/bindings/arm/coresight.txt          | 52
> >>>> ++++++++++++++++++++++
> >>>>   1 file changed, 52 insertions(+)
> >>>>
> >>>> diff --git a/Documentation/devicetree/bindings/arm/coresight.txt
> >>>> b/Documentation/devicetree/bindings/arm/coresight.txt
> >>>> index 15ac8e8..cdd84d0 100644
> >>>> --- a/Documentation/devicetree/bindings/arm/coresight.txt
> >>>> +++ b/Documentation/devicetree/bindings/arm/coresight.txt
> >>>> @@ -39,6 +39,8 @@ its hardware characteristcs.
> >>>>
> >>>>                - System Trace Macrocell:
> >>>>                        "arm,coresight-stm", "arm,primecell"; [1]
> >>>> +             - Coresight Address Translation Unit (CATU)
> >>>> +                     "arm, coresight-catu", "arm,primecell";
> >>>
> >>>
> >>> spurious space               ^
> >
> >
> > Thanks for spotting, will fix it.
> >
> >>>
> >>>>
> >>>>        * reg: physical base address and length of the register
> >>>>          set(s) of the component.
> >>>> @@ -86,6 +88,9 @@ its hardware characteristcs.
> >>>>        * arm,buffer-size: size of contiguous buffer space for TMC ETR
> >>>>         (embedded trace router)
> >>>>
> >>>> +* Optional property for CATU :
> >>>> +     * interrupts : Exactly one SPI may be listed for reporting the
> >>>> address
> >>>> +       error
> >>>
> >>>
> >>> Somewhere you need to define the ports for the CATU.
> >
> >
> > The ports are defined common to all the coresight components. Would you
> > like it to be added just for the CATU ?
> 
> Yeah, that's probably how we got into this problem with the port
> numbering in the first place.
> 
> 
> >>>>   Example:
> >>>>
> >>>> @@ -118,6 +123,35 @@ Example:
> >>>>                };
> >>>>        };
> >>>>
> >>>> +     etr@20070000 {
> >>>> +             compatible = "arm,coresight-tmc", "arm,primecell";
> >>>> +             reg = <0 0x20070000 0 0x1000>;
> >>>> +
> >>>> +                     /* input port */
> >>>> +                     port@0 {
> >>>> +                             reg =  <0>;
> >>>> +                             etr_in_port: endpoint {
> >>>> +                                     slave-mode;
> >>>> +                                     remote-endpoint =
> >>>> <&replicator2_out_port0>;
> >>>> +                             };
> >>>> +                     };
> >>>> +
> >>>> +                     /* CATU link represented by output port */
> >>>> +                     port@1 {
> >>>> +                             reg = <0>;
> >>>
> >>>
> >>> While common in the Coresight bindings, having unit-address and reg not
> >>> match is an error. Mathieu and I discussed this a bit as dtc now warns
> >>> on these.
> >>>
> >>> Either reg should be 1 here, or 'ports' needs to be split into input and
> >>> output ports. My preference would be the former, but Mathieu objected to
> >>> this not reflecting the the h/w numbering.
> >>
> >>
> >> Suzuki, as we discuss this is related to your work on revamping CS
> >> bindings for ACPI.  Until that gets done and to move forward with this
> >> set I suggest you abide to Rob's request.
> >
> >
> > Ok, I can change it to <1>, as we don't expect any other output port for an
> > ETR.
> 
> Better let Mathieu confirm he's okay with the first option because he
> wasn't okay with changing the port reg when we discussed. But maybe
> that was just on existing things like TPIU.

I'm good with this one as it is a new component and doesn't change anything
that people could be relying on.

> 
> Rob

^ permalink raw reply

* Re: [PATCH 2/2] media: i2c: mt9t112: Add device tree support
From: jacopo mondi @ 2018-05-14 14:30 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: Jacopo Mondi, hans.verkuil, mchehab, robh+dt, linux-media,
	devicetree, linux-kernel
In-Reply-To: <20180507093219.hrhaliadccaytenj@valkosipuli.retiisi.org.uk>

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

Hi Sakari,

On Mon, May 07, 2018 at 12:32:19PM +0300, Sakari Ailus wrote:
> Hi Jacopo,
>
> On Wed, Apr 25, 2018 at 01:00:14PM +0200, Jacopo Mondi wrote:

[snip]

> >  static int mt9t112_probe(struct i2c_client *client,
> >  			 const struct i2c_device_id *did)
> >  {
> >  	struct mt9t112_priv *priv;
> >  	int ret;
> >
> > -	if (!client->dev.platform_data) {
> > +	if (!client->dev.of_node && !client->dev.platform_data) {
> >  		dev_err(&client->dev, "mt9t112: missing platform data!\n");
> >  		return -EINVAL;
> >  	}
> > @@ -1081,23 +1118,39 @@ static int mt9t112_probe(struct i2c_client *client,
> >  	if (!priv)
> >  		return -ENOMEM;
> >
> > -	priv->info = client->dev.platform_data;
> >  	priv->init_done = false;
> > -
> > -	v4l2_i2c_subdev_init(&priv->subdev, client, &mt9t112_subdev_ops);
> > -
> > -	priv->clk = devm_clk_get(&client->dev, "extclk");
> > -	if (PTR_ERR(priv->clk) == -ENOENT) {
> > +	priv->dev = &client->dev;
> > +
> > +	if (client->dev.platform_data) {
> > +		priv->info = client->dev.platform_data;
> > +
> > +		priv->clk = devm_clk_get(&client->dev, "extclk");
>
> extclk needs to be documented in DT binding documentation.
>
> > +		if (PTR_ERR(priv->clk) == -ENOENT) {
> > +			priv->clk = NULL;
> > +		} else if (IS_ERR(priv->clk)) {
> > +			dev_err(&client->dev,
> > +				"Unable to get clock \"extclk\"\n");
> > +			return PTR_ERR(priv->clk);
> > +		}
> > +	} else {
> > +		/*
> > +		 * External clock frequencies != 24MHz are only supported
> > +		 * for non-OF systems.
> > +		 */
>
> Shouldn't you actually set the frequency? Or perhaps even better to check
> it, and use assigned-clocks and assigned-clock-rates properties?
>

I might be confused, but my intention was to use an external clock
reference, with a configurable frequency only in the platform data use
case. As you can see in this 'else' branch, in OF case, the priv->clk
field is null, and all the PLL and clock computations are performed
assuming a 24MHz input clock.

In my opinion, as the driver when running on OF systems does not
get any reference to 'extclk' clock, it should not be documented in
bindings. Do you agree?

Thanks
   j

> >  		priv->clk = NULL;
> > -	} else if (IS_ERR(priv->clk)) {
> > -		dev_err(&client->dev, "Unable to get clock \"extclk\"\n");
> > -		return PTR_ERR(priv->clk);
> > +		priv->info = &mt9t112_default_pdata_24MHz;
> > +
> > +		ret = mt9t112_parse_dt(priv);
> > +		if (ret)
> > +			return ret;
> >  	}
> >
> > -	priv->standby_gpio = devm_gpiod_get_optional(&client->dev, "standby",
> > +	v4l2_i2c_subdev_init(&priv->subdev, client, &mt9t112_subdev_ops);
> > +
> > +	priv->standby_gpio = devm_gpiod_get_optional(&client->dev, "powerdown",
> >  						     GPIOD_OUT_HIGH);
> >  	if (IS_ERR(priv->standby_gpio)) {
> > -		dev_err(&client->dev, "Unable to get gpio \"standby\"\n");
> > +		dev_err(&client->dev, "Unable to get gpio \"powerdown\"\n");
> >  		return PTR_ERR(priv->standby_gpio);
> >  	}
> >
> > @@ -1124,9 +1177,19 @@ static const struct i2c_device_id mt9t112_id[] = {
> >  };
> >  MODULE_DEVICE_TABLE(i2c, mt9t112_id);
> >
> > +#if IS_ENABLED(CONFIG_OF)
> > +static const struct of_device_id mt9t112_of_match[] = {
> > +	{ .compatible = "micron,mt9t111", },
> > +	{ .compatible = "micron,mt9t112", },
> > +	{ /* sentinel */ },
> > +};
> > +MODULE_DEVICE_TABLE(of, mt9t112_of_match);
> > +#endif
> > +
> >  static struct i2c_driver mt9t112_i2c_driver = {
> >  	.driver = {
> >  		.name = "mt9t112",
> > +		.of_match_table = of_match_ptr(mt9t112_of_match),
>
> No need to use of_match_ptr().
>
> >  	},
> >  	.probe    = mt9t112_probe,
> >  	.remove   = mt9t112_remove,
>
> --
> Sakari Ailus
> e-mail: sakari.ailus@iki.fi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH 6/7] clk: meson: axg: document bindings for the audio clock controller
From: Jerome Brunet @ 2018-05-14 14:16 UTC (permalink / raw)
  To: Rob Herring
  Cc: Neil Armstrong, Carlo Caione, Kevin Hilman, Michael Turquette,
	Stephen Boyd, linux-amlogic, linux-clk, devicetree, linux-kernel
In-Reply-To: <20180501143715.GA20023@rob-hp-laptop>

On Tue, 2018-05-01 at 09:37 -0500, Rob Herring wrote:
> On Wed, Apr 25, 2018 at 06:33:03PM +0200, Jerome Brunet wrote:
> > Add documentation for the device tree bindings of the audio clock
> > controller of the A113 based SoCs
> > 
> 
> The preferred subject prefix for bindings is 'dt/bindings: clock: ...'

Noted, I suppose meant 'dt-bindings: clocks: ..." which seems to used half of
the times
The rest use 'clk: {platform}: ...'

> 
> > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> > ---
> >  .../bindings/clock/amlogic,axg-audio-clkc.txt      | 56 ++++++++++++++++++++++
> >  1 file changed, 56 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/clock/amlogic,axg-audio-clkc.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/clock/amlogic,axg-audio-clkc.txt b/Documentation/devicetree/bindings/clock/amlogic,axg-audio-clkc.txt
> > new file mode 100644
> > index 000000000000..1b989ceda567
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/clock/amlogic,axg-audio-clkc.txt
> > @@ -0,0 +1,56 @@
> > +* Amlogic AXG Audio Clock Controllers
> > +
> > +The Amlogic AXG audio clock controller generates and supplies clock to the
> > +other elements of the audio subsystem, such as fifos, i2s, spdif and pdm
> > +devices.
> > +
> > +Required Properties:
> > +
> > +- compatible	: should be "amlogic,axg-audio-clkc" for the A113X and A113D
> > +- reg		: physical base address of the clock controller and length of
> > +		  memory mapped region.
> > +- clocks	: a list of phandle + clock-specifier pairs for the clocks listed
> > +		  in clock-names.
> > +- clock-names	: must contain the following:
> > +		  * "pclk" - Main peripheral bus clock
> > +		  may contain the following:
> > +		  * "mst_in[0-7]" - 8 input plls to generate clock signals
> 
> Unless the sources don't exist, then you should always have the clocks. 
> It should not be based on whether you are using them or not.

This is not really the point. The parent controller may or may not provide some
of the PLL. This was the case during the development of the controller when the
HIFI pll was not available simply because there was no driver for it.

Whatever the reason for an input clock to be missing, this clock controller may
continue to serve its purpose even if some input plls are not provided/available
which is why these are not "mandatory"

> 
> > +		  * "slv_sclk[0-9]" - 10 slave bit clocks provided by external
> > +				      components.
> > +		  * "slv_lrclk[0-9]" - 10 slave sample clocks provided by external
> > +				       components.
> 
> I'm guessing these may come from pins that aren't connected, so being 
> optional is fine.
> 
> > +- reset		: phandle of the internal reset line
> 
> resets
> 
> > +- #clock-cells	: should be 1.
> > +
> > +Each clock is assigned an identifier and client nodes can use this identifier
> > +to specify the clock which they consume. All available clocks are defined as
> > +preprocessor macros in the dt-bindings/clock/axg-audio-clkc.h header and can be
> > +used in device tree sources.
> > +
> > +Example:
> > +
> > +clkc_audio: clock-controller {
> 
> Needs a unit address.
> 
> > +	compatible = "amlogic,axg-audio-clkc";
> > +	reg = <0x0 0x0 0x0 0xb4>;
> > +	#clock-cells = <1>;
> > +
> > +	clocks = <&clkc CLKID_AUDIO>,
> > +		 <&clkc CLKID_MPLL0>,
> > +		 <&clkc CLKID_MPLL1>,
> > +		 <&clkc CLKID_MPLL2>,
> > +		 <&clkc CLKID_MPLL3>,
> > +		 <&clkc CLKID_HIFI_PLL>,
> > +		 <&clkc CLKID_FCLK_DIV3>,
> > +		 <&clkc CLKID_FCLK_DIV4>,
> > +		 <&clkc CLKID_GP0_PLL>;
> > +	clock-names = "pclk",
> > +		      "mst_in0",
> > +		      "mst_in1",
> > +		      "mst_in2",
> > +		      "mst_in3",
> > +		      "mst_in4",
> > +		      "mst_in5",
> > +		      "mst_in6",
> > +		      "mst_in7";
> > +	resets = <&reset RESET_AUDIO>;
> > +};
> > -- 
> > 2.14.3
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe devicetree" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [RFC] ARM: dts: imx7s: Fix the port reg properties
From: Fabio Estevam @ 2018-05-14 14:09 UTC (permalink / raw)
  To: shawnguo; +Cc: Fabio Estevam, devicetree, robh+dt, linux-arm-kernel

From: Fabio Estevam <fabio.estevam@nxp.com>

The 'reg' property must match the unit address, so fix them
to avoid the following DTC warnings with W=1:

arch/arm/boot/dts/imx7d-cl-som-imx7.dtb: Warning (graph_port): /replicator/ports/port@2: graph node unit address error, expected "0"
arch/arm/boot/dts/imx7d-cl-som-imx7.dtb: Warning (graph_port): /soc/funnel@30041000/ports/port@2: graph node unit address error, expected "0"
arch/arm/boot/dts/imx7d-cl-som-imx7.dtb: Warning (graph_port): /soc/funnel@30083000/ports/port@2: graph node unit address error, expected "0"
arch/arm/boot/dts/imx7d-cl-som-imx7.dtb: Warning (graph_port): /soc/etf@30084000/ports/port@1: graph node unit address error, expected "0"

Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
---
Hi Rob,

Is this the correct way to fix these warnings?

 arch/arm/boot/dts/imx7s.dtsi | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
index 7d33993..55b6241 100644
--- a/arch/arm/boot/dts/imx7s.dtsi
+++ b/arch/arm/boot/dts/imx7s.dtsi
@@ -126,7 +126,7 @@
 
 			/* replicator input port */
 			port@2 {
-				reg = <0>;
+				reg = <2>;
 				replicator_in_port0: endpoint {
 					slave-mode;
 					remote-endpoint = <&etf_out_port>;
@@ -183,7 +183,7 @@
 
 				/* funnel output port */
 				port@2 {
-					reg = <0>;
+					reg = <2>;
 					ca_funnel_out_port0: endpoint {
 						remote-endpoint = <&hugo_funnel_in_port0>;
 					};
@@ -234,7 +234,7 @@
 				};
 
 				port@2 {
-					reg = <0>;
+					reg = <2>;
 					hugo_funnel_out_port0: endpoint {
 						remote-endpoint = <&etf_in_port>;
 					};
@@ -263,7 +263,7 @@
 				};
 
 				port@1 {
-					reg = <0>;
+					reg = <1>;
 					etf_out_port: endpoint {
 						remote-endpoint = <&replicator_in_port0>;
 					};
-- 
2.7.4

^ permalink raw reply related

* [PATCH v3 2/2] phy: add Rockchip Innosilicon hdmi phy
From: Heiko Stuebner @ 2018-05-14 13:57 UTC (permalink / raw)
  To: kishon
  Cc: mark.rutland, devicetree, Heiko Stuebner, linux-rockchip, robh+dt,
	robin.murphy, linux-arm-kernel, zhengyang
In-Reply-To: <20180514135711.19519-1-heiko@sntech.de>

From: Zheng Yang <zhengyang@rock-chips.com>

Add a driver for the Innosilicon hdmi phy used on rk3228/rk3229
and rk3328 socs from Rockchip.

Signed-off-by: Zheng Yang <zhengyang@rock-chips.com>
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
Tested-by: Robin Murphy <robin.murphy@arm.com>
---
changes in v3:
- included real recalc_rate for rk3228
- claim both refclks and keep the refpclk on
  to make rk3328 happy for now
- use SPDX identifier
changes in v2:
- prevent overflow in tmdsclk calculation
  as reported by Martin Cerveny
- use unsigned long for all tmdsclk rate uses
- simplify tmds rate calculation

 drivers/phy/rockchip/Kconfig                  |    7 +
 drivers/phy/rockchip/Makefile                 |    1 +
 drivers/phy/rockchip/phy-rockchip-inno-hdmi.c | 1275 +++++++++++++++++
 3 files changed, 1283 insertions(+)
 create mode 100644 drivers/phy/rockchip/phy-rockchip-inno-hdmi.c

diff --git a/drivers/phy/rockchip/Kconfig b/drivers/phy/rockchip/Kconfig
index 0e15119ddfc6..5753bdd75975 100644
--- a/drivers/phy/rockchip/Kconfig
+++ b/drivers/phy/rockchip/Kconfig
@@ -15,6 +15,13 @@ config PHY_ROCKCHIP_EMMC
 	help
 	  Enable this to support the Rockchip EMMC PHY.
 
+config PHY_ROCKCHIP_INNO_HDMI
+	tristate "Rockchip INNO HDMI PHY Driver"
+	depends on (ARCH_ROCKCHIP || COMPILE_TEST) && OF
+	select GENERIC_PHY
+	help
+	  Enable this to support the Rockchip Innosilicon HDMI PHY.
+
 config PHY_ROCKCHIP_INNO_USB2
 	tristate "Rockchip INNO USB2PHY Driver"
 	depends on (ARCH_ROCKCHIP || COMPILE_TEST) && OF
diff --git a/drivers/phy/rockchip/Makefile b/drivers/phy/rockchip/Makefile
index 7f149d989046..fd21cbaf40dd 100644
--- a/drivers/phy/rockchip/Makefile
+++ b/drivers/phy/rockchip/Makefile
@@ -1,6 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 obj-$(CONFIG_PHY_ROCKCHIP_DP)		+= phy-rockchip-dp.o
 obj-$(CONFIG_PHY_ROCKCHIP_EMMC)		+= phy-rockchip-emmc.o
+obj-$(CONFIG_PHY_ROCKCHIP_INNO_HDMI)	+= phy-rockchip-inno-hdmi.o
 obj-$(CONFIG_PHY_ROCKCHIP_INNO_USB2)	+= phy-rockchip-inno-usb2.o
 obj-$(CONFIG_PHY_ROCKCHIP_PCIE)		+= phy-rockchip-pcie.o
 obj-$(CONFIG_PHY_ROCKCHIP_TYPEC)	+= phy-rockchip-typec.o
diff --git a/drivers/phy/rockchip/phy-rockchip-inno-hdmi.c b/drivers/phy/rockchip/phy-rockchip-inno-hdmi.c
new file mode 100644
index 000000000000..b6bb1a8b863e
--- /dev/null
+++ b/drivers/phy/rockchip/phy-rockchip-inno-hdmi.c
@@ -0,0 +1,1275 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2017 Rockchip Electronics Co. Ltd.
+ *
+ * Author: Zheng Yang <zhengyang@rock-chips.com>
+ *         Heiko Stuebner <heiko@sntech.de>
+ */
+
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/nvmem-consumer.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/phy/phy.h>
+#include <linux/slab.h>
+
+#define UPDATE(x, h, l)		(((x) << (l)) & GENMASK((h), (l)))
+
+/* REG: 0x00 */
+#define RK3228_PRE_PLL_REFCLK_SEL_PCLK			BIT(0)
+/* REG: 0x01 */
+#define RK3228_BYPASS_RXSENSE_EN			BIT(2)
+#define RK3228_BYPASS_PWRON_EN				BIT(1)
+#define RK3228_BYPASS_PLLPD_EN				BIT(0)
+/* REG: 0x02 */
+#define RK3228_BYPASS_PDATA_EN				BIT(4)
+#define RK3228_PDATAEN_DISABLE				BIT(0)
+/* REG: 0x03 */
+#define RK3228_BYPASS_AUTO_TERM_RES_CAL			BIT(7)
+#define RK3228_AUTO_TERM_RES_CAL_SPEED_14_8(x)		UPDATE(x, 6, 0)
+/* REG: 0x04 */
+#define RK3228_AUTO_TERM_RES_CAL_SPEED_7_0(x)		UPDATE(x, 7, 0)
+/* REG: 0xaa */
+#define RK3228_POST_PLL_CTRL_MANUAL			BIT(0)
+/* REG: 0xe0 */
+#define RK3228_POST_PLL_POWER_DOWN			BIT(5)
+#define RK3228_PRE_PLL_POWER_DOWN			BIT(4)
+#define RK3228_RXSENSE_CLK_CH_ENABLE			BIT(3)
+#define RK3228_RXSENSE_DATA_CH2_ENABLE			BIT(2)
+#define RK3228_RXSENSE_DATA_CH1_ENABLE			BIT(1)
+#define RK3228_RXSENSE_DATA_CH0_ENABLE			BIT(0)
+/* REG: 0xe1 */
+#define RK3228_BANDGAP_ENABLE				BIT(4)
+#define RK3228_TMDS_DRIVER_ENABLE			GENMASK(3, 0)
+/* REG: 0xe2 */
+#define RK3228_PRE_PLL_FB_DIV_8_MASK			BIT(7)
+#define RK3228_PRE_PLL_FB_DIV_8(x)			UPDATE((x) >> 8, 7, 7)
+#define RK3228_PCLK_VCO_DIV_5_MASK			BIT(5)
+#define RK3228_PCLK_VCO_DIV_5(x)			UPDATE(x, 5, 5)
+#define RK3228_PRE_PLL_PRE_DIV_MASK			GENMASK(4, 0)
+#define RK3228_PRE_PLL_PRE_DIV(x)			UPDATE(x, 4, 0)
+/* REG: 0xe3 */
+#define RK3228_PRE_PLL_FB_DIV_7_0(x)			UPDATE(x, 7, 0)
+/* REG: 0xe4 */
+#define RK3228_PRE_PLL_PCLK_DIV_B_MASK			GENMASK(6, 5)
+#define RK3228_PRE_PLL_PCLK_DIV_B_SHIFT			5
+#define RK3228_PRE_PLL_PCLK_DIV_B(x)			UPDATE(x, 6, 5)
+#define RK3228_PRE_PLL_PCLK_DIV_A_MASK			GENMASK(4, 0)
+#define RK3228_PRE_PLL_PCLK_DIV_A(x)			UPDATE(x, 4, 0)
+/* REG: 0xe5 */
+#define RK3228_PRE_PLL_PCLK_DIV_C_MASK			GENMASK(6, 5)
+#define RK3228_PRE_PLL_PCLK_DIV_C(x)			UPDATE(x, 6, 5)
+#define RK3228_PRE_PLL_PCLK_DIV_D_MASK			GENMASK(4, 0)
+#define RK3228_PRE_PLL_PCLK_DIV_D(x)			UPDATE(x, 4, 0)
+/* REG: 0xe6 */
+#define RK3228_PRE_PLL_TMDSCLK_DIV_C_MASK		GENMASK(5, 4)
+#define RK3228_PRE_PLL_TMDSCLK_DIV_C(x)			UPDATE(x, 5, 4)
+#define RK3228_PRE_PLL_TMDSCLK_DIV_A_MASK		GENMASK(3, 2)
+#define RK3228_PRE_PLL_TMDSCLK_DIV_A(x)			UPDATE(x, 3, 2)
+#define RK3228_PRE_PLL_TMDSCLK_DIV_B_MASK		GENMASK(1, 0)
+#define RK3228_PRE_PLL_TMDSCLK_DIV_B(x)			UPDATE(x, 1, 0)
+/* REG: 0xe8 */
+#define RK3228_PRE_PLL_LOCK_STATUS			BIT(0)
+/* REG: 0xe9 */
+#define RK3228_POST_PLL_POST_DIV_ENABLE			UPDATE(3, 7, 6)
+#define RK3228_POST_PLL_PRE_DIV_MASK			GENMASK(4, 0)
+#define RK3228_POST_PLL_PRE_DIV(x)			UPDATE(x, 4, 0)
+/* REG: 0xea */
+#define RK3228_POST_PLL_FB_DIV_7_0(x)			UPDATE(x, 7, 0)
+/* REG: 0xeb */
+#define RK3228_POST_PLL_FB_DIV_8_MASK			BIT(7)
+#define RK3228_POST_PLL_FB_DIV_8(x)			UPDATE((x) >> 8, 7, 7)
+#define RK3228_POST_PLL_POST_DIV_MASK			GENMASK(5, 4)
+#define RK3228_POST_PLL_POST_DIV(x)			UPDATE(x, 5, 4)
+#define RK3228_POST_PLL_LOCK_STATUS			BIT(0)
+/* REG: 0xee */
+#define RK3228_TMDS_CH_TA_ENABLE			GENMASK(7, 4)
+/* REG: 0xef */
+#define RK3228_TMDS_CLK_CH_TA(x)			UPDATE(x, 7, 6)
+#define RK3228_TMDS_DATA_CH2_TA(x)			UPDATE(x, 5, 4)
+#define RK3228_TMDS_DATA_CH1_TA(x)			UPDATE(x, 3, 2)
+#define RK3228_TMDS_DATA_CH0_TA(x)			UPDATE(x, 1, 0)
+/* REG: 0xf0 */
+#define RK3228_TMDS_DATA_CH2_PRE_EMPHASIS_MASK		GENMASK(5, 4)
+#define RK3228_TMDS_DATA_CH2_PRE_EMPHASIS(x)		UPDATE(x, 5, 4)
+#define RK3228_TMDS_DATA_CH1_PRE_EMPHASIS_MASK		GENMASK(3, 2)
+#define RK3228_TMDS_DATA_CH1_PRE_EMPHASIS(x)		UPDATE(x, 3, 2)
+#define RK3228_TMDS_DATA_CH0_PRE_EMPHASIS_MASK		GENMASK(1, 0)
+#define RK3228_TMDS_DATA_CH0_PRE_EMPHASIS(x)		UPDATE(x, 1, 0)
+/* REG: 0xf1 */
+#define RK3228_TMDS_CLK_CH_OUTPUT_SWING(x)		UPDATE(x, 7, 4)
+#define RK3228_TMDS_DATA_CH2_OUTPUT_SWING(x)		UPDATE(x, 3, 0)
+/* REG: 0xf2 */
+#define RK3228_TMDS_DATA_CH1_OUTPUT_SWING(x)		UPDATE(x, 7, 4)
+#define RK3228_TMDS_DATA_CH0_OUTPUT_SWING(x)		UPDATE(x, 3, 0)
+
+/* REG: 0x01 */
+#define RK3328_BYPASS_RXSENSE_EN			BIT(2)
+#define RK3328_BYPASS_POWERON_EN			BIT(1)
+#define RK3328_BYPASS_PLLPD_EN				BIT(0)
+/* REG: 0x02 */
+#define RK3328_INT_POL_HIGH				BIT(7)
+#define RK3328_BYPASS_PDATA_EN				BIT(4)
+#define RK3328_PDATA_EN					BIT(0)
+/* REG:0x05 */
+#define RK3328_INT_TMDS_CLK(x)				UPDATE(x, 7, 4)
+#define RK3328_INT_TMDS_D2(x)				UPDATE(x, 3, 0)
+/* REG:0x07 */
+#define RK3328_INT_TMDS_D1(x)				UPDATE(x, 7, 4)
+#define RK3328_INT_TMDS_D0(x)				UPDATE(x, 3, 0)
+/* for all RK3328_INT_TMDS_*, ESD_DET as defined in 0xc8-0xcb */
+#define RK3328_INT_AGND_LOW_PULSE_LOCKED		BIT(3)
+#define RK3328_INT_RXSENSE_LOW_PULSE_LOCKED		BIT(2)
+#define RK3328_INT_VSS_AGND_ESD_DET			BIT(1)
+#define RK3328_INT_AGND_VSS_ESD_DET			BIT(0)
+/* REG: 0xa0 */
+#define RK3328_PCLK_VCO_DIV_5_MASK			BIT(1)
+#define RK3328_PCLK_VCO_DIV_5(x)			UPDATE(x, 1, 1)
+#define RK3328_PRE_PLL_POWER_DOWN			BIT(0)
+/* REG: 0xa1 */
+#define RK3328_PRE_PLL_PRE_DIV_MASK			GENMASK(5, 0)
+#define RK3328_PRE_PLL_PRE_DIV(x)			UPDATE(x, 5, 0)
+/* REG: 0xa2 */
+/* unset means center spread */
+#define RK3328_SPREAD_SPECTRUM_MOD_DOWN			BIT(7)
+#define RK3328_SPREAD_SPECTRUM_MOD_DISABLE		BIT(6)
+#define RK3328_PRE_PLL_FRAC_DIV_DISABLE			UPDATE(3, 5, 4)
+#define RK3328_PRE_PLL_FB_DIV_11_8_MASK			GENMASK(3, 0)
+#define RK3328_PRE_PLL_FB_DIV_11_8(x)			UPDATE((x) >> 8, 3, 0)
+/* REG: 0xa3 */
+#define RK3328_PRE_PLL_FB_DIV_7_0(x)			UPDATE(x, 7, 0)
+/* REG: 0xa4*/
+#define RK3328_PRE_PLL_TMDSCLK_DIV_C_MASK		GENMASK(1, 0)
+#define RK3328_PRE_PLL_TMDSCLK_DIV_C(x)			UPDATE(x, 1, 0)
+#define RK3328_PRE_PLL_TMDSCLK_DIV_B_MASK		GENMASK(3, 2)
+#define RK3328_PRE_PLL_TMDSCLK_DIV_B(x)			UPDATE(x, 3, 2)
+#define RK3328_PRE_PLL_TMDSCLK_DIV_A_MASK		GENMASK(5, 4)
+#define RK3328_PRE_PLL_TMDSCLK_DIV_A(x)			UPDATE(x, 5, 4)
+/* REG: 0xa5 */
+#define RK3328_PRE_PLL_PCLK_DIV_B_SHIFT			5
+#define RK3328_PRE_PLL_PCLK_DIV_B_MASK			GENMASK(6, 5)
+#define RK3328_PRE_PLL_PCLK_DIV_B(x)			UPDATE(x, 6, 5)
+#define RK3328_PRE_PLL_PCLK_DIV_A_MASK			GENMASK(4, 0)
+#define RK3328_PRE_PLL_PCLK_DIV_A(x)			UPDATE(x, 4, 0)
+/* REG: 0xa6 */
+#define RK3328_PRE_PLL_PCLK_DIV_C_SHIFT			5
+#define RK3328_PRE_PLL_PCLK_DIV_C_MASK			GENMASK(6, 5)
+#define RK3328_PRE_PLL_PCLK_DIV_C(x)			UPDATE(x, 6, 5)
+#define RK3328_PRE_PLL_PCLK_DIV_D_MASK			GENMASK(4, 0)
+#define RK3328_PRE_PLL_PCLK_DIV_D(x)			UPDATE(x, 4, 0)
+/* REG: 0xa9 */
+#define RK3328_PRE_PLL_LOCK_STATUS			BIT(0)
+/* REG: 0xaa */
+#define RK3328_POST_PLL_POST_DIV_ENABLE			GENMASK(3, 2)
+#define RK3328_POST_PLL_REFCLK_SEL_TMDS			BIT(1)
+#define RK3328_POST_PLL_POWER_DOWN			BIT(0)
+/* REG:0xab */
+#define RK3328_POST_PLL_FB_DIV_8(x)			UPDATE((x >> 8), 7, 7)
+#define RK3328_POST_PLL_PRE_DIV(x)			UPDATE(x, 4, 0)
+/* REG: 0xac */
+#define RK3328_POST_PLL_FB_DIV_7_0(x)			UPDATE(x, 7, 0)
+/* REG: 0xad */
+#define RK3328_POST_PLL_POST_DIV_MASK			GENMASK(1, 0)
+#define RK3328_POST_PLL_POST_DIV_2			0x0
+#define RK3328_POST_PLL_POST_DIV_4			0x1
+#define RK3328_POST_PLL_POST_DIV_8			0x3
+/* REG: 0xaf */
+#define RK3328_POST_PLL_LOCK_STATUS			BIT(0)
+/* REG: 0xb0 */
+#define RK3328_BANDGAP_ENABLE				BIT(2)
+/* REG: 0xb2 */
+#define RK3328_TMDS_CLK_DRIVER_EN			BIT(3)
+#define RK3328_TMDS_D2_DRIVER_EN			BIT(2)
+#define RK3328_TMDS_D1_DRIVER_EN			BIT(1)
+#define RK3328_TMDS_D0_DRIVER_EN			BIT(0)
+#define RK3328_TMDS_DRIVER_ENABLE		(RK3328_TMDS_CLK_DRIVER_EN | \
+						RK3328_TMDS_D2_DRIVER_EN | \
+						RK3328_TMDS_D1_DRIVER_EN | \
+						RK3328_TMDS_D0_DRIVER_EN)
+/* REG:0xc5 */
+#define RK3328_BYPASS_TERM_RESISTOR_CALIB		BIT(7)
+#define RK3328_TERM_RESISTOR_CALIB_SPEED_14_8(x)	UPDATE((x) >> 8, 6, 0)
+/* REG:0xc6 */
+#define RK3328_TERM_RESISTOR_CALIB_SPEED_7_0(x)		UPDATE(x, 7, 9)
+/* REG:0xc7 */
+#define RK3328_TERM_RESISTOR_50				UPDATE(0, 2, 1)
+#define RK3328_TERM_RESISTOR_62_5			UPDATE(1, 2, 1)
+#define RK3328_TERM_RESISTOR_75				UPDATE(2, 2, 1)
+#define RK3328_TERM_RESISTOR_100			UPDATE(3, 2, 1)
+/* REG 0xc8 - 0xcb */
+#define RK3328_ESD_DETECT_MASK				GENMASK(7, 6)
+#define RK3328_ESD_DETECT_340MV				(0 << 6)
+#define RK3328_ESD_DETECT_280MV				(1 << 6)
+#define RK3328_ESD_DETECT_260MV				(2 << 6)
+#define RK3328_ESD_DETECT_240MV				(3 << 6)
+/* resistors can be used in parallel */
+#define RK3328_TMDS_TERM_RESIST_MASK			GENMASK(5, 0)
+#define RK3328_TMDS_TERM_RESIST_75			BIT(5)
+#define RK3328_TMDS_TERM_RESIST_150			BIT(4)
+#define RK3328_TMDS_TERM_RESIST_300			BIT(3)
+#define RK3328_TMDS_TERM_RESIST_600			BIT(2)
+#define RK3328_TMDS_TERM_RESIST_1000			BIT(1)
+#define RK3328_TMDS_TERM_RESIST_2000			BIT(0)
+/* REG: 0xd1 */
+#define RK3328_PRE_PLL_FRAC_DIV_23_16(x)		UPDATE((x) >> 16, 7, 0)
+/* REG: 0xd2 */
+#define RK3328_PRE_PLL_FRAC_DIV_15_8(x)			UPDATE((x) >> 8, 7, 0)
+/* REG: 0xd3 */
+#define RK3328_PRE_PLL_FRAC_DIV_7_0(x)			UPDATE(x, 7, 0)
+
+struct inno_hdmi_phy_drv_data;
+
+struct inno_hdmi_phy {
+	struct device *dev;
+	struct regmap *regmap;
+	int irq;
+
+	struct phy *phy;
+	struct clk *sysclk;
+	struct clk *refoclk;
+	struct clk *refpclk;
+
+	/* platform data */
+	struct inno_hdmi_phy_drv_data *plat_data;
+	int chip_version;
+
+	/* clk provider */
+	struct clk_hw hw;
+	struct clk *phyclk;
+	unsigned long pixclock;
+};
+
+struct pre_pll_config {
+	unsigned long pixclock;
+	unsigned long tmdsclock;
+	u8 prediv;
+	u16 fbdiv;
+	u8 tmds_div_a;
+	u8 tmds_div_b;
+	u8 tmds_div_c;
+	u8 pclk_div_a;
+	u8 pclk_div_b;
+	u8 pclk_div_c;
+	u8 pclk_div_d;
+	u8 vco_div_5_en;
+	u32 fracdiv;
+};
+
+struct post_pll_config {
+	unsigned long tmdsclock;
+	u8 prediv;
+	u16 fbdiv;
+	u8 postdiv;
+	u8 version;
+};
+
+struct phy_config {
+	unsigned long	tmdsclock;
+	u8		regs[14];
+};
+
+struct inno_hdmi_phy_ops {
+	int (*init)(struct inno_hdmi_phy *inno);
+	int (*power_on)(struct inno_hdmi_phy *inno,
+			const struct post_pll_config *cfg,
+			const struct phy_config *phy_cfg);
+	void (*power_off)(struct inno_hdmi_phy *inno);
+};
+
+struct inno_hdmi_phy_drv_data {
+	const struct inno_hdmi_phy_ops	*ops;
+	const struct clk_ops		*clk_ops;
+	const struct phy_config		*phy_cfg_table;
+};
+
+static const struct pre_pll_config pre_pll_cfg_table[] = {
+	{ 27000000,  27000000, 1,  90, 3, 2, 2, 10, 3, 3, 4, 0, 0},
+	{ 27000000,  33750000, 1,  90, 1, 3, 3, 10, 3, 3, 4, 0, 0},
+	{ 40000000,  40000000, 1,  80, 2, 2, 2, 12, 2, 2, 2, 0, 0},
+	{ 59341000,  59341000, 1,  98, 3, 1, 2,  1, 3, 3, 4, 0, 0xE6AE6B},
+	{ 59400000,  59400000, 1,  99, 3, 1, 1,  1, 3, 3, 4, 0, 0},
+	{ 59341000,  74176250, 1,  98, 0, 3, 3,  1, 3, 3, 4, 0, 0xE6AE6B},
+	{ 59400000,  74250000, 1,  99, 1, 2, 2,  1, 3, 3, 4, 0, 0},
+	{ 74176000,  74176000, 1,  98, 1, 2, 2,  1, 2, 3, 4, 0, 0xE6AE6B},
+	{ 74250000,  74250000, 1,  99, 1, 2, 2,  1, 2, 3, 4, 0, 0},
+	{ 74176000,  92720000, 4, 494, 1, 2, 2,  1, 3, 3, 4, 0, 0x816817},
+	{ 74250000,  92812500, 4, 495, 1, 2, 2,  1, 3, 3, 4, 0, 0},
+	{148352000, 148352000, 1,  98, 1, 1, 1,  1, 2, 2, 2, 0, 0xE6AE6B},
+	{148500000, 148500000, 1,  99, 1, 1, 1,  1, 2, 2, 2, 0, 0},
+	{148352000, 185440000, 4, 494, 0, 2, 2,  1, 3, 2, 2, 0, 0x816817},
+	{148500000, 185625000, 4, 495, 0, 2, 2,  1, 3, 2, 2, 0, 0},
+	{296703000, 296703000, 1,  98, 0, 1, 1,  1, 0, 2, 2, 0, 0xE6AE6B},
+	{297000000, 297000000, 1,  99, 0, 1, 1,  1, 0, 2, 2, 0, 0},
+	{296703000, 370878750, 4, 494, 1, 2, 0,  1, 3, 1, 1, 0, 0x816817},
+	{297000000, 371250000, 4, 495, 1, 2, 0,  1, 3, 1, 1, 0, 0},
+	{593407000, 296703500, 1,  98, 0, 1, 1,  1, 0, 2, 1, 0, 0xE6AE6B},
+	{594000000, 297000000, 1,  99, 0, 1, 1,  1, 0, 2, 1, 0, 0},
+	{593407000, 370879375, 4, 494, 1, 2, 0,  1, 3, 1, 1, 1, 0x816817},
+	{594000000, 371250000, 4, 495, 1, 2, 0,  1, 3, 1, 1, 1, 0},
+	{593407000, 593407000, 1,  98, 0, 2, 0,  1, 0, 1, 1, 0, 0xE6AE6B},
+	{594000000, 594000000, 1,  99, 0, 2, 0,  1, 0, 1, 1, 0, 0},
+	{ /* sentinel */ }
+};
+
+static const struct post_pll_config post_pll_cfg_table[] = {
+	{33750000,  1, 40, 8, 1},
+	{33750000,  1, 80, 8, 2},
+	{74250000,  1, 40, 8, 1},
+	{74250000, 18, 80, 8, 2},
+	{148500000, 2, 40, 4, 3},
+	{297000000, 4, 40, 2, 3},
+	{594000000, 8, 40, 1, 3},
+	{ /* sentinel */ }
+};
+
+static const struct phy_config rk3228_phy_cfg[] = {
+	{	165000000, {
+			0xaa, 0x00, 0x44, 0x44, 0x00, 0x00, 0x00, 0x00, 0x00,
+			0x00, 0x00, 0x00, 0x00, 0x00,
+		},
+	}, {
+		340000000, {
+			0xaa, 0x15, 0x6a, 0xaa, 0x00, 0x00, 0x00, 0x00, 0x00,
+			0x00, 0x00, 0x00, 0x00, 0x00,
+		},
+	}, {
+		594000000, {
+			0xaa, 0x15, 0x7a, 0xaa, 0x00, 0x00, 0x00, 0x00, 0x00,
+			0x00, 0x00, 0x00, 0x00, 0x00,
+		},
+	}, { /* sentinel */ },
+};
+
+static const struct phy_config rk3328_phy_cfg[] = {
+	{	165000000, {
+			0x07, 0x0a, 0x0a, 0x0a, 0x00, 0x00, 0x08, 0x08, 0x08,
+			0x00, 0xac, 0xcc, 0xcc, 0xcc,
+		},
+	}, {
+		340000000, {
+			0x0b, 0x0d, 0x0d, 0x0d, 0x07, 0x15, 0x08, 0x08, 0x08,
+			0x3f, 0xac, 0xcc, 0xcd, 0xdd,
+		},
+	}, {
+		594000000, {
+			0x10, 0x1a, 0x1a, 0x1a, 0x07, 0x15, 0x08, 0x08, 0x08,
+			0x00, 0xac, 0xcc, 0xcc, 0xcc,
+		},
+	}, { /* sentinel */ },
+};
+
+static inline struct inno_hdmi_phy *to_inno_hdmi_phy(struct clk_hw *hw)
+{
+	return container_of(hw, struct inno_hdmi_phy, hw);
+}
+
+/*
+ * The register description of the IP block does not use any distinct names
+ * but instead the databook simply numbers the registers in one-increments.
+ * As the registers are obviously 32bit sized, the inno_* functions
+ * translate the databook register names to the actual registers addresses.
+ */
+static inline void inno_write(struct inno_hdmi_phy *inno, u32 reg, u8 val)
+{
+	regmap_write(inno->regmap, reg * 4, val);
+}
+
+static inline u8 inno_read(struct inno_hdmi_phy *inno, u32 reg)
+{
+	u32 val;
+
+	regmap_read(inno->regmap, reg * 4, &val);
+
+	return val;
+}
+
+static inline void inno_update_bits(struct inno_hdmi_phy *inno, u8 reg,
+				    u8 mask, u8 val)
+{
+	regmap_update_bits(inno->regmap, reg * 4, mask, val);
+}
+
+#define inno_poll(inno, reg, val, cond, sleep_us, timeout_us) \
+	regmap_read_poll_timeout(inno->regmap, reg * 4, val, cond, \
+				 sleep_us, timeout_us)
+
+static unsigned long inno_hdmi_phy_get_tmdsclk(struct inno_hdmi_phy *inno,
+					       unsigned long rate)
+{
+	int bus_width = phy_get_bus_width(inno->phy);
+
+	switch (bus_width) {
+	case 4:
+	case 5:
+	case 6:
+	case 10:
+	case 12:
+	case 16:
+		return (u64)rate * bus_width / 8;
+	default:
+		return rate;
+	}
+}
+
+static irqreturn_t inno_hdmi_phy_rk3328_hardirq(int irq, void *dev_id)
+{
+	struct inno_hdmi_phy *inno = dev_id;
+	int intr_stat1, intr_stat2, intr_stat3;
+
+	intr_stat1 = inno_read(inno, 0x04);
+	intr_stat2 = inno_read(inno, 0x06);
+	intr_stat3 = inno_read(inno, 0x08);
+
+	if (intr_stat1)
+		inno_write(inno, 0x04, intr_stat1);
+	if (intr_stat2)
+		inno_write(inno, 0x06, intr_stat2);
+	if (intr_stat3)
+		inno_write(inno, 0x08, intr_stat3);
+
+	if (intr_stat1 || intr_stat2 || intr_stat3)
+		return IRQ_WAKE_THREAD;
+
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t inno_hdmi_phy_rk3328_irq(int irq, void *dev_id)
+{
+	struct inno_hdmi_phy *inno = dev_id;
+
+	inno_update_bits(inno, 0x02, RK3328_PDATA_EN, 0);
+	usleep_range(9, 10);
+	inno_update_bits(inno, 0x02, RK3328_PDATA_EN, RK3328_PDATA_EN);
+
+	return IRQ_HANDLED;
+}
+
+static int inno_hdmi_phy_power_on(struct phy *phy)
+{
+	struct inno_hdmi_phy *inno = phy_get_drvdata(phy);
+	const struct post_pll_config *cfg = post_pll_cfg_table;
+	const struct phy_config *phy_cfg = inno->plat_data->phy_cfg_table;
+	unsigned long tmdsclock = inno_hdmi_phy_get_tmdsclk(inno,
+							    inno->pixclock);
+	int ret;
+
+	if (!tmdsclock) {
+		dev_err(inno->dev, "TMDS clock is zero!\n");
+		return -EINVAL;
+	}
+
+	if (!inno->plat_data->ops->power_on)
+		return -EINVAL;
+
+	for (; cfg->tmdsclock != 0; cfg++)
+		if (tmdsclock <= cfg->tmdsclock &&
+		    cfg->version & inno->chip_version)
+			break;
+
+	for (; phy_cfg->tmdsclock != 0; phy_cfg++)
+		if (tmdsclock <= phy_cfg->tmdsclock)
+			break;
+
+	if (cfg->tmdsclock == 0 || phy_cfg->tmdsclock == 0)
+		return -EINVAL;
+
+	dev_dbg(inno->dev, "Inno HDMI PHY Power On\n");
+
+	ret = clk_prepare_enable(inno->phyclk);
+	if (ret)
+		return ret;
+
+	ret = inno->plat_data->ops->power_on(inno, cfg, phy_cfg);
+	if (ret) {
+		clk_disable_unprepare(inno->phyclk);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int inno_hdmi_phy_power_off(struct phy *phy)
+{
+	struct inno_hdmi_phy *inno = phy_get_drvdata(phy);
+
+	if (!inno->plat_data->ops->power_off)
+		return -EINVAL;
+
+	inno->plat_data->ops->power_off(inno);
+
+	clk_disable_unprepare(inno->phyclk);
+
+	dev_dbg(inno->dev, "Inno HDMI PHY Power Off\n");
+
+	return 0;
+}
+
+static const struct phy_ops inno_hdmi_phy_ops = {
+	.owner = THIS_MODULE,
+	.power_on = inno_hdmi_phy_power_on,
+	.power_off = inno_hdmi_phy_power_off,
+};
+
+static const struct pre_pll_config *inno_hdmi_phy_get_pre_pll_cfg(
+				struct inno_hdmi_phy *inno, unsigned long rate)
+{
+	const struct pre_pll_config *cfg = pre_pll_cfg_table;
+	unsigned long tmdsclock = inno_hdmi_phy_get_tmdsclk(inno, rate);
+
+	for (; cfg->pixclock != 0; cfg++)
+		if (cfg->pixclock == rate && cfg->tmdsclock == tmdsclock)
+			break;
+
+	if (cfg->pixclock == 0)
+		return ERR_PTR(-EINVAL);
+
+	return cfg;
+}
+
+static int inno_hdmi_phy_rk3228_clk_is_prepared(struct clk_hw *hw)
+{
+	struct inno_hdmi_phy *inno = to_inno_hdmi_phy(hw);
+	u8 status;
+
+	status = inno_read(inno, 0xe0) & RK3228_PRE_PLL_POWER_DOWN;
+	return status ? 0 : 1;
+}
+
+static int inno_hdmi_phy_rk3228_clk_prepare(struct clk_hw *hw)
+{
+	struct inno_hdmi_phy *inno = to_inno_hdmi_phy(hw);
+
+	inno_update_bits(inno, 0xe0, RK3228_PRE_PLL_POWER_DOWN, 0);
+	return 0;
+}
+
+static void inno_hdmi_phy_rk3228_clk_unprepare(struct clk_hw *hw)
+{
+	struct inno_hdmi_phy *inno = to_inno_hdmi_phy(hw);
+
+	inno_update_bits(inno, 0xe0, RK3228_PRE_PLL_POWER_DOWN,
+				     RK3228_PRE_PLL_POWER_DOWN);
+}
+
+static unsigned long inno_hdmi_phy_rk3228_clk_recalc_rate(struct clk_hw *hw,
+						   unsigned long parent_rate)
+{
+	struct inno_hdmi_phy *inno = to_inno_hdmi_phy(hw);
+	u8 nd, no_a, no_b, no_d;
+	u64 vco;
+	u16 nf;
+
+	nd = inno_read(inno, 0xe2) & RK3228_PRE_PLL_PRE_DIV_MASK;
+	nf = (inno_read(inno, 0xe2) & RK3228_PRE_PLL_FB_DIV_8_MASK) << 1;
+	nf |= inno_read(inno, 0xe3);
+	vco = parent_rate * nf;
+
+	if (inno_read(inno, 0xe2) & RK3228_PCLK_VCO_DIV_5_MASK) {
+		do_div(vco, nd * 5);
+	} else {
+		no_a = inno_read(inno, 0xe4) & RK3228_PRE_PLL_PCLK_DIV_A_MASK;
+		if (!no_a)
+			no_a = 1;
+		no_b = inno_read(inno, 0xe4) & RK3228_PRE_PLL_PCLK_DIV_B_MASK;
+		no_b >>= RK3228_PRE_PLL_PCLK_DIV_B_SHIFT;
+		no_b += 2;
+		no_d = inno_read(inno, 0xe5) & RK3228_PRE_PLL_PCLK_DIV_D_MASK;
+
+		do_div(vco, (nd * (no_a == 1 ? no_b : no_a) * no_d * 2));
+	}
+
+	inno->pixclock = vco;
+
+	dev_dbg(inno->dev, "%s rate %lu\n", __func__, inno->pixclock);
+
+	return vco;
+}
+
+static long inno_hdmi_phy_rk3228_clk_round_rate(struct clk_hw *hw,
+				unsigned long rate, unsigned long *parent_rate)
+{
+	const struct pre_pll_config *cfg = pre_pll_cfg_table;
+
+	for (; cfg->pixclock != 0; cfg++)
+		if (cfg->pixclock == rate && !cfg->fracdiv)
+			break;
+
+	if (cfg->pixclock == 0)
+		return -EINVAL;
+
+	return cfg->pixclock;
+}
+
+static int inno_hdmi_phy_rk3228_clk_set_rate(struct clk_hw *hw,
+				unsigned long rate, unsigned long parent_rate)
+{
+	struct inno_hdmi_phy *inno = to_inno_hdmi_phy(hw);
+	const struct pre_pll_config *cfg = pre_pll_cfg_table;
+	unsigned long tmdsclock = inno_hdmi_phy_get_tmdsclk(inno, rate);
+	u32 v;
+	int ret;
+
+	dev_dbg(inno->dev, "%s rate %lu tmdsclk %lu\n",
+		__func__, rate, tmdsclock);
+
+	cfg = inno_hdmi_phy_get_pre_pll_cfg(inno, rate);
+	if (IS_ERR(cfg))
+		return PTR_ERR(cfg);
+
+	/* Power down PRE-PLL */
+	inno_update_bits(inno, 0xe0, RK3228_PRE_PLL_POWER_DOWN,
+			     RK3228_PRE_PLL_POWER_DOWN);
+
+	inno_update_bits(inno, 0xe2, RK3228_PRE_PLL_FB_DIV_8_MASK |
+			     RK3228_PCLK_VCO_DIV_5_MASK |
+			     RK3228_PRE_PLL_PRE_DIV_MASK,
+			     RK3228_PRE_PLL_FB_DIV_8(cfg->fbdiv) |
+			     RK3228_PCLK_VCO_DIV_5(cfg->vco_div_5_en) |
+			     RK3228_PRE_PLL_PRE_DIV(cfg->prediv));
+	inno_write(inno, 0xe3, RK3228_PRE_PLL_FB_DIV_7_0(cfg->fbdiv));
+	inno_update_bits(inno, 0xe4, RK3228_PRE_PLL_PCLK_DIV_B_MASK |
+			     RK3228_PRE_PLL_PCLK_DIV_A_MASK,
+			     RK3228_PRE_PLL_PCLK_DIV_B(cfg->pclk_div_b) |
+			     RK3228_PRE_PLL_PCLK_DIV_A(cfg->pclk_div_a));
+	inno_update_bits(inno, 0xe5, RK3228_PRE_PLL_PCLK_DIV_C_MASK |
+			     RK3228_PRE_PLL_PCLK_DIV_D_MASK,
+			     RK3228_PRE_PLL_PCLK_DIV_C(cfg->pclk_div_c) |
+			     RK3228_PRE_PLL_PCLK_DIV_D(cfg->pclk_div_d));
+	inno_update_bits(inno, 0xe6, RK3228_PRE_PLL_TMDSCLK_DIV_C_MASK |
+			     RK3228_PRE_PLL_TMDSCLK_DIV_A_MASK |
+			     RK3228_PRE_PLL_TMDSCLK_DIV_B_MASK,
+			     RK3228_PRE_PLL_TMDSCLK_DIV_C(cfg->tmds_div_c) |
+			     RK3228_PRE_PLL_TMDSCLK_DIV_A(cfg->tmds_div_a) |
+			     RK3228_PRE_PLL_TMDSCLK_DIV_B(cfg->tmds_div_b));
+
+	/* Power up PRE-PLL */
+	inno_update_bits(inno, 0xe0, RK3228_PRE_PLL_POWER_DOWN, 0);
+
+	/* Wait for Pre-PLL lock */
+	ret = inno_poll(inno, 0xe8, v, v & RK3228_PRE_PLL_LOCK_STATUS,
+			100, 100000);
+	if (ret) {
+		dev_err(inno->dev, "Pre-PLL locking failed\n");
+		return ret;
+	}
+
+	inno->pixclock = rate;
+
+	return 0;
+}
+
+static const struct clk_ops inno_hdmi_phy_rk3228_clk_ops = {
+	.prepare = inno_hdmi_phy_rk3228_clk_prepare,
+	.unprepare = inno_hdmi_phy_rk3228_clk_unprepare,
+	.is_prepared = inno_hdmi_phy_rk3228_clk_is_prepared,
+	.recalc_rate = inno_hdmi_phy_rk3228_clk_recalc_rate,
+	.round_rate = inno_hdmi_phy_rk3228_clk_round_rate,
+	.set_rate = inno_hdmi_phy_rk3228_clk_set_rate,
+};
+
+static int inno_hdmi_phy_rk3328_clk_is_prepared(struct clk_hw *hw)
+{
+	struct inno_hdmi_phy *inno = to_inno_hdmi_phy(hw);
+	u8 status;
+
+	status = inno_read(inno, 0xa0) & RK3328_PRE_PLL_POWER_DOWN;
+	return status ? 0 : 1;
+}
+
+static int inno_hdmi_phy_rk3328_clk_prepare(struct clk_hw *hw)
+{
+	struct inno_hdmi_phy *inno = to_inno_hdmi_phy(hw);
+
+	inno_update_bits(inno, 0xa0, RK3328_PRE_PLL_POWER_DOWN, 0);
+	return 0;
+}
+
+static void inno_hdmi_phy_rk3328_clk_unprepare(struct clk_hw *hw)
+{
+	struct inno_hdmi_phy *inno = to_inno_hdmi_phy(hw);
+
+	inno_update_bits(inno, 0xa0, RK3328_PRE_PLL_POWER_DOWN,
+				     RK3328_PRE_PLL_POWER_DOWN);
+}
+
+static unsigned long inno_hdmi_phy_rk3328_clk_recalc_rate(struct clk_hw *hw,
+						   unsigned long parent_rate)
+{
+	struct inno_hdmi_phy *inno = to_inno_hdmi_phy(hw);
+	unsigned long frac;
+	u8 nd, no_a, no_b, no_c, no_d;
+	u64 vco;
+	u16 nf;
+
+	nd = inno_read(inno, 0xa1) & RK3328_PRE_PLL_PRE_DIV_MASK;
+	nf = ((inno_read(inno, 0xa2) & RK3328_PRE_PLL_FB_DIV_11_8_MASK) << 8);
+	nf |= inno_read(inno, 0xa3);
+	vco = parent_rate * nf;
+
+	if (!(inno_read(inno, 0xa2) & RK3328_PRE_PLL_FRAC_DIV_DISABLE)) {
+		frac = inno_read(inno, 0xd3) |
+		       (inno_read(inno, 0xd2) << 8) |
+		       (inno_read(inno, 0xd1) << 16);
+		vco += DIV_ROUND_CLOSEST(parent_rate * frac, (1 << 24));
+	}
+
+	if (inno_read(inno, 0xa0) & RK3328_PCLK_VCO_DIV_5_MASK) {
+		do_div(vco, nd * 5);
+	} else {
+		no_a = inno_read(inno, 0xa5) & RK3328_PRE_PLL_PCLK_DIV_A_MASK;
+		no_b = inno_read(inno, 0xa5) & RK3328_PRE_PLL_PCLK_DIV_B_MASK;
+		no_b >>= RK3328_PRE_PLL_PCLK_DIV_B_SHIFT;
+		no_b += 2;
+		no_c = inno_read(inno, 0xa6) & RK3328_PRE_PLL_PCLK_DIV_C_MASK;
+		no_c >>= RK3328_PRE_PLL_PCLK_DIV_C_SHIFT;
+		no_c = 1 << no_c;
+		no_d = inno_read(inno, 0xa6) & RK3328_PRE_PLL_PCLK_DIV_D_MASK;
+
+		do_div(vco, (nd * (no_a == 1 ? no_b : no_a) * no_d * 2));
+	}
+
+	inno->pixclock = vco;
+	dev_dbg(inno->dev, "%s rate %lu\n", __func__, inno->pixclock);
+
+	return vco;
+}
+
+static long inno_hdmi_phy_rk3328_clk_round_rate(struct clk_hw *hw,
+				unsigned long rate, unsigned long *parent_rate)
+{
+	const struct pre_pll_config *cfg = pre_pll_cfg_table;
+
+	for (; cfg->pixclock != 0; cfg++)
+		if (cfg->pixclock == rate)
+			break;
+
+	if (cfg->pixclock == 0)
+		return -EINVAL;
+
+	return cfg->pixclock;
+}
+
+static int inno_hdmi_phy_rk3328_clk_set_rate(struct clk_hw *hw,
+				unsigned long rate, unsigned long parent_rate)
+{
+	struct inno_hdmi_phy *inno = to_inno_hdmi_phy(hw);
+	const struct pre_pll_config *cfg = pre_pll_cfg_table;
+	unsigned long tmdsclock = inno_hdmi_phy_get_tmdsclk(inno, rate);
+	u32 val;
+	int ret;
+
+	dev_dbg(inno->dev, "%s rate %lu tmdsclk %lu\n",
+		__func__, rate, tmdsclock);
+
+	cfg = inno_hdmi_phy_get_pre_pll_cfg(inno, rate);
+	if (IS_ERR(cfg))
+		return PTR_ERR(cfg);
+
+	inno_update_bits(inno, 0xa0, RK3328_PRE_PLL_POWER_DOWN,
+				     RK3328_PRE_PLL_POWER_DOWN);
+
+	/* Configure pre-pll */
+	inno_update_bits(inno, 0xa0, RK3228_PCLK_VCO_DIV_5_MASK,
+				     RK3228_PCLK_VCO_DIV_5(cfg->vco_div_5_en));
+	inno_write(inno, 0xa1, RK3328_PRE_PLL_PRE_DIV(cfg->prediv));
+
+	val = RK3328_SPREAD_SPECTRUM_MOD_DISABLE;
+	if (!cfg->fracdiv)
+		val |= RK3328_PRE_PLL_FRAC_DIV_DISABLE;
+	inno_write(inno, 0xa2, RK3328_PRE_PLL_FB_DIV_11_8(cfg->fbdiv) | val);
+	inno_write(inno, 0xa3, RK3328_PRE_PLL_FB_DIV_7_0(cfg->fbdiv));
+	inno_write(inno, 0xa5, RK3328_PRE_PLL_PCLK_DIV_A(cfg->pclk_div_a) |
+			       RK3328_PRE_PLL_PCLK_DIV_B(cfg->pclk_div_b));
+	inno_write(inno, 0xa6, RK3328_PRE_PLL_PCLK_DIV_C(cfg->pclk_div_c) |
+			       RK3328_PRE_PLL_PCLK_DIV_D(cfg->pclk_div_d));
+	inno_write(inno, 0xa4, RK3328_PRE_PLL_TMDSCLK_DIV_C(cfg->tmds_div_c) |
+			       RK3328_PRE_PLL_TMDSCLK_DIV_A(cfg->tmds_div_a) |
+			       RK3328_PRE_PLL_TMDSCLK_DIV_B(cfg->tmds_div_b));
+	inno_write(inno, 0xd3, RK3328_PRE_PLL_FRAC_DIV_7_0(cfg->fracdiv));
+	inno_write(inno, 0xd2, RK3328_PRE_PLL_FRAC_DIV_15_8(cfg->fracdiv));
+	inno_write(inno, 0xd1, RK3328_PRE_PLL_FRAC_DIV_23_16(cfg->fracdiv));
+
+	inno_update_bits(inno, 0xa0, RK3328_PRE_PLL_POWER_DOWN, 0);
+
+	/* Wait for Pre-PLL lock */
+	ret = inno_poll(inno, 0xa9, val, val & RK3328_PRE_PLL_LOCK_STATUS,
+			1000, 10000);
+	if (ret) {
+		dev_err(inno->dev, "Pre-PLL locking failed\n");
+		return ret;
+	}
+
+	inno->pixclock = rate;
+
+	return 0;
+}
+
+static const struct clk_ops inno_hdmi_phy_rk3328_clk_ops = {
+	.prepare = inno_hdmi_phy_rk3328_clk_prepare,
+	.unprepare = inno_hdmi_phy_rk3328_clk_unprepare,
+	.is_prepared = inno_hdmi_phy_rk3328_clk_is_prepared,
+	.recalc_rate = inno_hdmi_phy_rk3328_clk_recalc_rate,
+	.round_rate = inno_hdmi_phy_rk3328_clk_round_rate,
+	.set_rate = inno_hdmi_phy_rk3328_clk_set_rate,
+};
+
+static int inno_hdmi_phy_clk_register(struct inno_hdmi_phy *inno)
+{
+	struct device *dev = inno->dev;
+	struct device_node *np = dev->of_node;
+	struct clk_init_data init;
+	const char *parent_name;
+	int ret;
+
+	parent_name = __clk_get_name(inno->refoclk);
+
+	init.parent_names = &parent_name;
+	init.num_parents = 1;
+	init.flags = 0;
+	init.name = "pin_hd20_pclk";
+	init.ops = inno->plat_data->clk_ops;
+
+	/* optional override of the clock name */
+	of_property_read_string(np, "clock-output-names", &init.name);
+
+	inno->hw.init = &init;
+
+	inno->phyclk = devm_clk_register(dev, &inno->hw);
+	if (IS_ERR(inno->phyclk)) {
+		ret = PTR_ERR(inno->phyclk);
+		dev_err(dev, "failed to register clock: %d\n", ret);
+		return ret;
+	}
+
+	ret = of_clk_add_provider(np, of_clk_src_simple_get, inno->phyclk);
+	if (ret) {
+		dev_err(dev, "failed to register clock provider: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int inno_hdmi_phy_rk3228_init(struct inno_hdmi_phy *inno)
+{
+	/*
+	 * Use phy internal register control
+	 * rxsense/poweron/pllpd/pdataen signal.
+	 */
+	inno_write(inno, 0x01, RK3228_BYPASS_RXSENSE_EN |
+			       RK3228_BYPASS_PWRON_EN |
+			       RK3228_BYPASS_PLLPD_EN);
+	inno_update_bits(inno, 0x02, RK3228_BYPASS_PDATA_EN,
+				     RK3228_BYPASS_PDATA_EN);
+
+	/* manual power down post-PLL */
+	inno_update_bits(inno, 0xaa, RK3228_POST_PLL_CTRL_MANUAL,
+				     RK3228_POST_PLL_CTRL_MANUAL);
+
+	inno->chip_version = 1;
+
+	return 0;
+}
+
+static int
+inno_hdmi_phy_rk3228_power_on(struct inno_hdmi_phy *inno,
+			      const struct post_pll_config *cfg,
+			      const struct phy_config *phy_cfg)
+{
+	int ret;
+	u32 v;
+
+	inno_update_bits(inno, 0x02, RK3228_PDATAEN_DISABLE,
+				     RK3228_PDATAEN_DISABLE);
+	inno_update_bits(inno, 0xe0, RK3228_PRE_PLL_POWER_DOWN |
+				     RK3228_POST_PLL_POWER_DOWN,
+				     RK3228_PRE_PLL_POWER_DOWN |
+				     RK3228_POST_PLL_POWER_DOWN);
+
+	/* Post-PLL update */
+	inno_update_bits(inno, 0xe9, RK3228_POST_PLL_PRE_DIV_MASK,
+				     RK3228_POST_PLL_PRE_DIV(cfg->prediv));
+	inno_update_bits(inno, 0xeb, RK3228_POST_PLL_FB_DIV_8_MASK,
+				     RK3228_POST_PLL_FB_DIV_8(cfg->fbdiv));
+	inno_write(inno, 0xea, RK3228_POST_PLL_FB_DIV_7_0(cfg->fbdiv));
+
+	if (cfg->postdiv == 1) {
+		inno_update_bits(inno, 0xe9, RK3228_POST_PLL_POST_DIV_ENABLE,
+					     0);
+	} else {
+		inno_update_bits(inno, 0xe9, RK3228_POST_PLL_POST_DIV_ENABLE,
+					     RK3228_POST_PLL_POST_DIV_ENABLE);
+		inno_update_bits(inno, 0xeb, RK3228_POST_PLL_POST_DIV_MASK,
+				RK3228_POST_PLL_POST_DIV(cfg->postdiv / 2 - 1));
+	}
+
+	for (v = 0; v < 4; v++)
+		inno_write(inno, 0xef + v, phy_cfg->regs[v]);
+
+	inno_update_bits(inno, 0xe0, RK3228_PRE_PLL_POWER_DOWN |
+				     RK3228_POST_PLL_POWER_DOWN, 0);
+	inno_update_bits(inno, 0xe1, RK3228_BANDGAP_ENABLE,
+				     RK3228_BANDGAP_ENABLE);
+	inno_update_bits(inno, 0xe1, RK3228_TMDS_DRIVER_ENABLE,
+				     RK3228_TMDS_DRIVER_ENABLE);
+
+	/* Wait for post PLL lock */
+	ret = inno_poll(inno, 0xeb, v, v & RK3228_POST_PLL_LOCK_STATUS,
+			100, 100000);
+	if (ret) {
+		dev_err(inno->dev, "Post-PLL locking failed\n");
+		return ret;
+	}
+
+	if (cfg->tmdsclock > 340000000)
+		msleep(100);
+
+	inno_update_bits(inno, 0x02, RK3228_PDATAEN_DISABLE, 0);
+	return 0;
+}
+
+static void inno_hdmi_phy_rk3228_power_off(struct inno_hdmi_phy *inno)
+{
+	inno_update_bits(inno, 0xe1, RK3228_TMDS_DRIVER_ENABLE, 0);
+	inno_update_bits(inno, 0xe1, RK3228_BANDGAP_ENABLE, 0);
+	inno_update_bits(inno, 0xe0, RK3228_POST_PLL_POWER_DOWN,
+				     RK3228_POST_PLL_POWER_DOWN);
+}
+
+static const struct inno_hdmi_phy_ops rk3228_hdmi_phy_ops = {
+	.init = inno_hdmi_phy_rk3228_init,
+	.power_on = inno_hdmi_phy_rk3228_power_on,
+	.power_off = inno_hdmi_phy_rk3228_power_off,
+};
+
+static int inno_hdmi_phy_rk3328_init(struct inno_hdmi_phy *inno)
+{
+	struct nvmem_cell *cell;
+	unsigned char *efuse_buf;
+	size_t len;
+
+	/*
+	 * Use phy internal register control
+	 * rxsense/poweron/pllpd/pdataen signal.
+	 */
+	inno_write(inno, 0x01, RK3328_BYPASS_RXSENSE_EN |
+			       RK3328_BYPASS_POWERON_EN |
+			       RK3328_BYPASS_PLLPD_EN);
+	inno_write(inno, 0x02, RK3328_INT_POL_HIGH | RK3328_BYPASS_PDATA_EN |
+			       RK3328_PDATA_EN);
+
+	/* Disable phy irq */
+	inno_write(inno, 0x05, 0);
+	inno_write(inno, 0x07, 0);
+
+	/* try to read the chip-version */
+	inno->chip_version = 1;
+	cell = nvmem_cell_get(inno->dev, "cpu-version");
+	if (IS_ERR(cell)) {
+		if (PTR_ERR(cell) == -EPROBE_DEFER)
+			return -EPROBE_DEFER;
+
+		return 0;
+	}
+
+	efuse_buf = nvmem_cell_read(cell, &len);
+	nvmem_cell_put(cell);
+
+	if (IS_ERR(efuse_buf))
+		return 0;
+	if (len == 1)
+		inno->chip_version = efuse_buf[0] + 1;
+	kfree(efuse_buf);
+
+	return 0;
+}
+
+static int
+inno_hdmi_phy_rk3328_power_on(struct inno_hdmi_phy *inno,
+			      const struct post_pll_config *cfg,
+			      const struct phy_config *phy_cfg)
+{
+	int ret;
+	u32 v;
+
+	inno_update_bits(inno, 0x02, RK3328_PDATA_EN, 0);
+	inno_update_bits(inno, 0xaa, RK3328_POST_PLL_POWER_DOWN,
+				     RK3328_POST_PLL_POWER_DOWN);
+
+	inno_write(inno, 0xac, RK3328_POST_PLL_FB_DIV_7_0(cfg->fbdiv));
+	if (cfg->postdiv == 1) {
+		inno_write(inno, 0xaa, RK3328_POST_PLL_REFCLK_SEL_TMDS);
+		inno_write(inno, 0xab, RK3328_POST_PLL_FB_DIV_8(cfg->fbdiv) |
+				       RK3328_POST_PLL_PRE_DIV(cfg->prediv));
+	} else {
+		v = (cfg->postdiv / 2) - 1;
+		v &= RK3328_POST_PLL_POST_DIV_MASK;
+		inno_write(inno, 0xad, v);
+		inno_write(inno, 0xab, RK3328_POST_PLL_FB_DIV_8(cfg->fbdiv) |
+				       RK3328_POST_PLL_PRE_DIV(cfg->prediv));
+		inno_write(inno, 0xaa, RK3328_POST_PLL_POST_DIV_ENABLE |
+				       RK3328_POST_PLL_REFCLK_SEL_TMDS);
+	}
+
+	for (v = 0; v < 14; v++)
+		inno_write(inno, 0xb5 + v, phy_cfg->regs[v]);
+
+	/* set ESD detection threshold for TMDS CLK, D2, D1 and D0 */
+	for (v = 0; v < 4; v++)
+		inno_update_bits(inno, 0xc8 + v, RK3328_ESD_DETECT_MASK,
+						 RK3328_ESD_DETECT_340MV);
+
+	if (phy_cfg->tmdsclock > 340000000) {
+		/* Set termination resistor to 100ohm */
+		v = clk_get_rate(inno->sysclk) / 100000;
+		inno_write(inno, 0xc5, RK3328_TERM_RESISTOR_CALIB_SPEED_14_8(v)
+				     | RK3328_BYPASS_TERM_RESISTOR_CALIB);
+		inno_write(inno, 0xc6, RK3328_TERM_RESISTOR_CALIB_SPEED_7_0(v));
+		inno_write(inno, 0xc7, RK3328_TERM_RESISTOR_100);
+		inno_update_bits(inno, 0xc5,
+				       RK3328_BYPASS_TERM_RESISTOR_CALIB, 0);
+	} else {
+		inno_write(inno, 0xc5, RK3328_BYPASS_TERM_RESISTOR_CALIB);
+
+		/* clk termination resistor is 50ohm (parallel resistors) */
+		if (phy_cfg->tmdsclock > 165000000)
+			inno_update_bits(inno, 0xc8,
+						RK3328_TMDS_TERM_RESIST_MASK,
+						RK3328_TMDS_TERM_RESIST_75 |
+						RK3328_TMDS_TERM_RESIST_150);
+
+		/* data termination resistor for D2, D1 and D0 is 150ohm */
+		for (v = 0; v < 3; v++)
+			inno_update_bits(inno, 0xc9 + v,
+						RK3328_TMDS_TERM_RESIST_MASK,
+						RK3328_TMDS_TERM_RESIST_150);
+	}
+
+	inno_update_bits(inno, 0xaa, RK3328_POST_PLL_POWER_DOWN, 0);
+	inno_update_bits(inno, 0xb0, RK3328_BANDGAP_ENABLE,
+				     RK3328_BANDGAP_ENABLE);
+	inno_update_bits(inno, 0xb2, RK3328_TMDS_DRIVER_ENABLE,
+				     RK3328_TMDS_DRIVER_ENABLE);
+
+	/* Wait for post PLL lock */
+	ret = inno_poll(inno, 0xaf, v, v & RK3328_POST_PLL_LOCK_STATUS,
+			1000, 10000);
+	if (ret) {
+		dev_err(inno->dev, "Post-PLL locking failed\n");
+		return ret;
+	}
+
+	if (phy_cfg->tmdsclock > 340000000)
+		msleep(100);
+
+	inno_update_bits(inno, 0x02, RK3328_PDATA_EN, RK3328_PDATA_EN);
+
+	/* Enable PHY IRQ */
+	inno_write(inno, 0x05, RK3328_INT_TMDS_CLK(RK3328_INT_VSS_AGND_ESD_DET)
+			     | RK3328_INT_TMDS_D2(RK3328_INT_VSS_AGND_ESD_DET));
+	inno_write(inno, 0x07, RK3328_INT_TMDS_D1(RK3328_INT_VSS_AGND_ESD_DET)
+			     | RK3328_INT_TMDS_D0(RK3328_INT_VSS_AGND_ESD_DET));
+	return 0;
+}
+
+static void inno_hdmi_phy_rk3328_power_off(struct inno_hdmi_phy *inno)
+{
+	inno_update_bits(inno, 0xb2, RK3328_TMDS_DRIVER_ENABLE, 0);
+	inno_update_bits(inno, 0xb0, RK3328_BANDGAP_ENABLE, 0);
+	inno_update_bits(inno, 0xaa, RK3328_POST_PLL_POWER_DOWN,
+				     RK3328_POST_PLL_POWER_DOWN);
+
+	/* Disable PHY IRQ */
+	inno_write(inno, 0x05, 0);
+	inno_write(inno, 0x07, 0);
+}
+
+static const struct inno_hdmi_phy_ops rk3328_hdmi_phy_ops = {
+	.init = inno_hdmi_phy_rk3328_init,
+	.power_on = inno_hdmi_phy_rk3328_power_on,
+	.power_off = inno_hdmi_phy_rk3328_power_off,
+};
+
+static const struct inno_hdmi_phy_drv_data rk3228_hdmi_phy_drv_data = {
+	.ops = &rk3228_hdmi_phy_ops,
+	.clk_ops = &inno_hdmi_phy_rk3228_clk_ops,
+	.phy_cfg_table = rk3228_phy_cfg,
+};
+
+static const struct inno_hdmi_phy_drv_data rk3328_hdmi_phy_drv_data = {
+	.ops = &rk3328_hdmi_phy_ops,
+	.clk_ops = &inno_hdmi_phy_rk3328_clk_ops,
+	.phy_cfg_table = rk3328_phy_cfg,
+};
+
+static const struct regmap_config inno_hdmi_phy_regmap_config = {
+	.reg_bits = 32,
+	.val_bits = 32,
+	.reg_stride = 4,
+	.max_register = 0x400,
+};
+
+static void inno_hdmi_phy_action(void *data)
+{
+	struct inno_hdmi_phy *inno = data;
+
+	clk_disable_unprepare(inno->refpclk);
+	clk_disable_unprepare(inno->sysclk);
+}
+
+static int inno_hdmi_phy_probe(struct platform_device *pdev)
+{
+	struct inno_hdmi_phy *inno;
+	const struct of_device_id *match;
+	struct phy_provider *phy_provider;
+	struct resource *res;
+	void __iomem *regs;
+	int ret;
+
+	inno = devm_kzalloc(&pdev->dev, sizeof(*inno), GFP_KERNEL);
+	if (!inno)
+		return -ENOMEM;
+
+	inno->dev = &pdev->dev;
+
+	match = of_match_device(inno->dev->driver->of_match_table, inno->dev);
+	inno->plat_data = (struct inno_hdmi_phy_drv_data *)match->data;
+	if (!inno->plat_data || !inno->plat_data->ops)
+		return -EINVAL;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	regs = devm_ioremap_resource(inno->dev, res);
+	if (IS_ERR(regs))
+		return PTR_ERR(regs);
+
+	inno->sysclk = devm_clk_get(inno->dev, "sysclk");
+	if (IS_ERR(inno->sysclk)) {
+		ret = PTR_ERR(inno->sysclk);
+		dev_err(inno->dev, "failed to get sysclk: %d\n", ret);
+		return ret;
+	}
+
+	inno->refpclk = devm_clk_get(inno->dev, "refpclk");
+	if (IS_ERR(inno->refpclk)) {
+		ret = PTR_ERR(inno->refpclk);
+		dev_err(inno->dev, "failed to get ref clock: %d\n", ret);
+		return ret;
+	}
+
+	inno->refoclk = devm_clk_get(inno->dev, "refoclk");
+	if (IS_ERR(inno->refoclk)) {
+		ret = PTR_ERR(inno->refoclk);
+		dev_err(inno->dev, "failed to get oscillator-ref clock: %d\n",
+				    ret);
+		return ret;
+	}
+
+	ret = clk_prepare_enable(inno->sysclk);
+	if (ret) {
+		dev_err(inno->dev, "Cannot enable inno phy sysclk: %d\n", ret);
+		return ret;
+	}
+
+	/*
+	 * Refpclk needs to be on, on at least the rk3328 for still
+	 * unknown reasons.
+	 */
+	ret = clk_prepare_enable(inno->refpclk);
+	if (ret) {
+		dev_err(inno->dev, "failed to enable refpclk\n");
+		clk_disable_unprepare(inno->sysclk);
+		return ret;
+	}
+
+	ret = devm_add_action_or_reset(inno->dev, inno_hdmi_phy_action,
+				       inno);
+	if (ret) {
+		clk_disable_unprepare(inno->refpclk);
+		clk_disable_unprepare(inno->sysclk);
+		return ret;
+	}
+
+	inno->regmap = devm_regmap_init_mmio(inno->dev, regs,
+					     &inno_hdmi_phy_regmap_config);
+	if (IS_ERR(inno->regmap))
+		return PTR_ERR(inno->regmap);
+
+	/* only the newer rk3328 hdmiphy has an interrupt */
+	inno->irq = platform_get_irq(pdev, 0);
+	if (inno->irq > 0) {
+		ret = devm_request_threaded_irq(inno->dev, inno->irq,
+					inno_hdmi_phy_rk3328_hardirq,
+					inno_hdmi_phy_rk3328_irq, IRQF_SHARED,
+					dev_name(inno->dev), inno);
+		if (ret)
+			return ret;
+	}
+
+	inno->phy = devm_phy_create(inno->dev, NULL, &inno_hdmi_phy_ops);
+	if (IS_ERR(inno->phy)) {
+		dev_err(inno->dev, "failed to create HDMI PHY\n");
+		return PTR_ERR(inno->phy);
+	}
+
+	phy_set_drvdata(inno->phy, inno);
+	phy_set_bus_width(inno->phy, 8);
+
+	if (inno->plat_data->ops->init) {
+		ret = inno->plat_data->ops->init(inno);
+		if (ret)
+			return ret;
+	}
+
+	ret = inno_hdmi_phy_clk_register(inno);
+	if (ret)
+		return ret;
+
+	phy_provider = devm_of_phy_provider_register(inno->dev,
+						     of_phy_simple_xlate);
+	if (IS_ERR(phy_provider)) {
+		dev_err(inno->dev, "failed to register PHY provider\n");
+		return PTR_ERR(phy_provider);
+	}
+
+	return 0;
+}
+
+static int inno_hdmi_phy_remove(struct platform_device *pdev)
+{
+	of_clk_del_provider(pdev->dev.of_node);
+
+	return 0;
+}
+
+static const struct of_device_id inno_hdmi_phy_of_match[] = {
+	{
+		.compatible = "rockchip,rk3228-hdmi-phy",
+		.data = &rk3228_hdmi_phy_drv_data
+	}, {
+		.compatible = "rockchip,rk3328-hdmi-phy",
+		.data = &rk3328_hdmi_phy_drv_data
+	}, { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, inno_hdmi_phy_of_match);
+
+static struct platform_driver inno_hdmi_phy_driver = {
+	.probe  = inno_hdmi_phy_probe,
+	.remove = inno_hdmi_phy_remove,
+	.driver = {
+		.name = "inno-hdmi-phy",
+		.of_match_table = inno_hdmi_phy_of_match,
+	},
+};
+module_platform_driver(inno_hdmi_phy_driver);
+
+MODULE_AUTHOR("Zheng Yang <zhengyang@rock-chips.com>");
+MODULE_DESCRIPTION("Innosilion HDMI 2.0 Transmitter PHY Driver");
+MODULE_LICENSE("GPL v2");
-- 
2.17.0

^ permalink raw reply related

* [PATCH v3 1/2] dt-bindings: add binding for Rockchip hdmi phy using an Innosilicon IP
From: Heiko Stuebner @ 2018-05-14 13:57 UTC (permalink / raw)
  To: kishon
  Cc: mark.rutland, devicetree, Heiko Stuebner, linux-rockchip, robh+dt,
	robin.murphy, linux-arm-kernel, zhengyang

From: Zheng Yang <zhengyang@rock-chips.com>

The phy is used so far in two Rockchip socs the rk3228 and the rk3328.

Signed-off-by: Zheng Yang <zhengyang@rock-chips.com>
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
Reviewed-by: Rob Herring <robh@kernel.org>
---
changes in v3:
- describe both refclks supplied to the phy block

 .../bindings/phy/phy-rockchip-inno-hdmi.txt   | 43 +++++++++++++++++++
 1 file changed, 43 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/phy-rockchip-inno-hdmi.txt

diff --git a/Documentation/devicetree/bindings/phy/phy-rockchip-inno-hdmi.txt b/Documentation/devicetree/bindings/phy/phy-rockchip-inno-hdmi.txt
new file mode 100644
index 000000000000..710cccd5ee56
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/phy-rockchip-inno-hdmi.txt
@@ -0,0 +1,43 @@
+ROCKCHIP HDMI PHY WITH INNO IP BLOCK
+
+Required properties:
+ - compatible : should be one of the listed compatibles:
+	* "rockchip,rk3228-hdmi-phy",
+	* "rockchip,rk3328-hdmi-phy";
+ - reg : Address and length of the hdmi phy control register set
+ - clocks : phandle + clock specifier for the phy clocks
+ - clock-names : string, clock name, must contain "sysclk" for system
+	  control and register configuration, "refoclk" for crystal-
+	  oscillator reference PLL clock input and "refpclk" for pclk-
+	  based refeference PLL clock input.
+ - #clock-cells: should be 0.
+ - clock-output-names : shall be the name for the output clock.
+ - interrupts : phandle + interrupt specified for the hdmiphy interrupt
+ - #phy-cells : must be 0. See ./phy-bindings.txt for details.
+
+Optional properties for rk3328-hdmi-phy:
+ - nvmem-cells = phandle + nvmem specifier for the cpu-version efuse
+ - nvmem-cell-names : "cpu-version" to read the chip version, required
+	  for adjustment to some frequency settings
+
+Example:
+	hdmi_phy: hdmi-phy@12030000 {
+		compatible = "rockchip,rk3228-hdmi-phy";
+		reg = <0x12030000 0x10000>;
+		#phy-cells = <0>;
+		clocks = <&cru PCLK_HDMI_PHY>, <&xin24m>, <&cru DCLK_HDMIPHY>;
+		clock-names = "sysclk", "refoclk", "refpclk";
+		#clock-cells = <0>;
+		clock-output-names = "hdmi_phy";
+		status = "disabled";
+	};
+
+Then the PHY can be used in other nodes such as:
+
+	hdmi: hdmi@200a0000 {
+		compatible = "rockchip,rk3228-dw-hdmi";
+		...
+		phys = <&hdmi_phy>;
+		phy-names = "hdmi";
+		...
+	};
-- 
2.17.0

^ permalink raw reply related

* Re: [PATCH 1/2] ARM64: dts: meson-gxbb: odroidc2: enable sdcard UHS modes
From: Jerome Brunet @ 2018-05-14 13:48 UTC (permalink / raw)
  To: Anand Moon
  Cc: Neil Armstrong, Rob Herring, Mark Rutland, Catalin Marinas,
	Will Deacon, Carlo Caione, Kevin Hilman, devicetree,
	linux-arm-kernel, linux-amlogic, linux-kernel
In-Reply-To: <20180501185902.3556-1-linux.amoon@gmail.com>

On Wed, 2018-05-02 at 00:29 +0530, Anand Moon wrote:
> Enable UHS modes for sdcard, on odroidc2.
> 
> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> ---
>  arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
> index 54954b314a45..c90f8b46c60c 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
> +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
> @@ -258,6 +258,11 @@
>  	cap-sd-highspeed;
>  	max-frequency = <100000000>;
>  	disable-wp;
> +	sd-uhs-sdr12;
> +	sd-uhs-sdr25;
> +	sd-uhs-sdr50;
> +	sd-uhs-sdr104;
> +	sd-uhs-ddr50;
>  
>  	cd-gpios = <&gpio CARD_6 GPIO_ACTIVE_HIGH>;
>  	cd-inverted;

Hi Anand,

I tried a few sdcards on the OC2 with your 2 patches.
Like with the libretech-cc, sdr104@200Mhz works "mostly", but, with some
sdcards, it does not - please see below. The same sdcards appear to be working
fine on my laptop.

This is something I have not been able to crack yet on the libretech-cc.

I'd suggest dropping sdr104 and keeping the max frequency at 100Mhz until we can
figure out the problem here.

With this changed:

Tested-by: Jerome Brunet <jbrunet@baylibre.com>

dd if=/dev/mmcblk1 of=/dev/zero bs=4M                                   
[  446.925817] mmc1: tuning execution failed: -5                               
[  446.956597] mmc1: tuning execution failed: -5                               
[  489.957810] print_req_error: I/O error, dev mmcblk1, sector 6654424         
[  490.141975] print_req_error: I/O error, dev mmcblk1, sector 6656440         
[  490.148304] print_req_error: I/O error, dev mmcblk1, sector 6656944         
[  490.349650] print_req_error: I/O error, dev mmcblk1, sector 6658456         
[  491.804382] print_req_error: I/O error, dev mmcblk1, sector 6747688         
[  492.281246] print_req_error: I/O error, dev mmcblk1, sector 6784992         
[  492.419034] print_req_error: I/O error, dev mmcblk1, sector 6785496         
[  492.865878] print_req_error: I/O error, dev mmcblk1, sector 6791800         
[  493.023809] print_req_error: I/O error, dev mmcblk1, sector 6792192         
[  493.024435] Buffer I/O error on dev mmcblk1, logical block 849024, async page
read                                                                         
[  493.217751] print_req_error: I/O error, dev mmcblk1, sector 6792808         
[  494.891779] mmc1: tuning execution failed: -5                               
[  495.374186] print_req_error: 3 callbacks suppressed                         
[  495.374193] print_req_error: I/O error, dev mmcblk1, sector 6854576         
[  495.767498] print_req_error: I/O error, dev mmcblk1, sector 6860000         
[  496.013104] print_req_error: I/O error, dev mmcblk1, sector 6863024         
[  496.223042] print_req_error: I/O error, dev mmcblk1, sector 6864032         
[  496.227003] print_req_error: I/O error, dev mmcblk1, sector 6864536         
[  496.375175] print_req_error: I/O error, dev mmcblk1, sector 6864176         
[  496.375806] Buffer I/O error on dev mmcblk1, logical block 858022, async page
read                                                                         
[  496.521229] print_req_error: I/O error, dev mmcblk1, sector 6864184         
[  496.521852] Buffer I/O error on dev mmcblk1, logical block 858023, async page
read                                                                         
[  503.596978] print_req_error: I/O error, dev mmcblk1, sector 6983312         
[  503.597606] Buffer I/O error on dev mmcblk1, logical block 872914, async page
read                                                                         
[  505.280621] print_req_error: I/O error, dev mmcblk1, sector 7004536         
[  505.281249] Buffer I/O error on dev mmcblk1, logical block 875567, async page
read                                                                         
[  507.372560] print_req_error: I/O error, dev mmcblk1, sector 7048696         
[  507.373192] Buffer I/O error on dev mmcblk1, logical block 881087, async page
read                                                                         
[  511.355248] print_req_error: I/O error, dev mmcblk1, sector 7131352         
[  511.355883] Buffer I/O error on dev mmcblk1, logical block 891419, async page
read                                                                         
[  511.369076] print_req_error: I/O error, dev mmcblk1, sector 7131352         
[  511.369694] Buffer I/O error on dev mmcblk1, logical block 891419, async page
read                                                                         
dd: error reading '/dev/mmcblk1': Input/output error                           
868+7 records in                       
868+7 records out                      
3651252224 bytes (3.7 GB, 3.4 GiB) copied, 66.7736 s, 54.7 MB/s   

^ permalink raw reply

* Re: [PATCH 5/6 v3] bus: fsl-mc: supoprt dma configure for devices on fsl-mc bus
From: Laurentiu Tudor @ 2018-05-14 13:39 UTC (permalink / raw)
  To: Nipun Gupta, robin.murphy-5wv7dgnIgG8@public.gmane.org,
	will.deacon-5wv7dgnIgG8@public.gmane.org,
	mark.rutland-5wv7dgnIgG8@public.gmane.org,
	catalin.marinas-5wv7dgnIgG8@public.gmane.org
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	stuyoder-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org,
	linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Leo Li,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	linux-pci-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	bhelgaas-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org,
	shawnguo-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	hch-jcswGhMUV9g@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <1524824826-29473-6-git-send-email-nipun.gupta-3arQi8VN3Tc@public.gmane.org>

Hi Nipun,

On 04/27/2018 01:27 PM, Nipun Gupta wrote:
> Signed-off-by: Nipun Gupta <nipun.gupta-3arQi8VN3Tc@public.gmane.org>
> ---
>   drivers/bus/fsl-mc/fsl-mc-bus.c | 16 ++++++++++++----
>   1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/bus/fsl-mc/fsl-mc-bus.c b/drivers/bus/fsl-mc/fsl-mc-bus.c
> index 5d8266c..624828b 100644
> --- a/drivers/bus/fsl-mc/fsl-mc-bus.c
> +++ b/drivers/bus/fsl-mc/fsl-mc-bus.c
> @@ -127,6 +127,16 @@ static int fsl_mc_bus_uevent(struct device *dev, struct kobj_uevent_env *env)
>   	return 0;
>   }
>
> +static int fsl_mc_dma_configure(struct device *dev)
> +{
> +	struct device *dma_dev = dev;
> +
> +	while (dev_is_fsl_mc(dma_dev))
> +		dma_dev = dma_dev->parent;
> +
> +	return of_dma_configure(dev, dma_dev->of_node, 0);
> +}
> +
>   static ssize_t modalias_show(struct device *dev, struct device_attribute *attr,
>   			     char *buf)
>   {
> @@ -148,6 +158,7 @@ struct bus_type fsl_mc_bus_type = {
>   	.name = "fsl-mc",
>   	.match = fsl_mc_bus_match,
>   	.uevent = fsl_mc_bus_uevent,
> +	.dma_configure  = fsl_mc_dma_configure,
>   	.dev_groups = fsl_mc_dev_groups,
>   };
>   EXPORT_SYMBOL_GPL(fsl_mc_bus_type);
> @@ -616,6 +627,7 @@ int fsl_mc_device_add(struct fsl_mc_obj_desc *obj_desc,
>   		mc_dev->icid = parent_mc_dev->icid;
>   		mc_dev->dma_mask = FSL_MC_DEFAULT_DMA_MASK;
>   		mc_dev->dev.dma_mask = &mc_dev->dma_mask;
> +		mc_dev->dev.coherent_dma_mask = mc_dev->dma_mask;

This change seems a bit unrelated to the patch subject. I wonder if it 
makes sense to split it it in a distinct patch.

---
Best Regards, Laurentiu

^ permalink raw reply

* Re: [PATCH 10/12] platform/early: implement support for early platform drivers
From: Rob Herring @ 2018-05-14 13:37 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Sekhar Nori, Kevin Hilman, David Lechner, Michael Turquette,
	Stephen Boyd, Arnd Bergmann, Greg Kroah-Hartman, Mark Rutland,
	Yoshinori Sato, Rich Felker, Andy Shevchenko, Marc Zyngier,
	Rafael J . Wysocki, Peter Rosin, Jiri Slaby, Thomas Gleixner,
	Daniel Lezcano, Geert Uytterhoeven, Magnus Damm, Johan Hovold
In-Reply-To: <20180511162028.20616-11-brgl@bgdev.pl>

On Fri, May 11, 2018 at 11:20 AM, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>
> This introduces the core part of support for early platform drivers
> and devices.
>

It looks like most of your prep patches are to separate the alloc and
init of platform devices because you are essentially making early
devices/drivers a sub-class. Maybe you could avoid doing that and
simplify things a bit. Comments below based on doing that...

> +/**
> + * struct early_platform_driver
> + *
> + * @pdrv: real platform driver associated with this early platform driver
> + * @list: list head for the list of early platform drivers
> + * @early_probe: early probe callback
> + */
> +struct early_platform_driver {
> +       struct platform_driver pdrv;
> +       struct list_head list;

Couldn't you use an existing list in driver_private until you move
over to the normal bus infra.

> +       int (*early_probe)(struct platform_device *);

Just add this to platform_driver.

> +};
> +
> +/**
> + * struct early_platform_device
> + *
> + * @pdev: real platform device associated with this early platform device
> + * @list: list head for the list of early platform devices
> + * @deferred: true if this device's early probe was deferred
> + * @deferred_drv: early platform driver with which this device was matched
> + */
> +struct early_platform_device {
> +       struct platform_device pdev;
> +       struct list_head list;

Use a list in device_private?

> +       bool deferred;
> +       struct early_platform_driver *deferred_drv;

Can't you use the existing deferred probe list?

Rob

^ permalink raw reply

* [PATCH v2] ARM: dts: imx6/7: Remove unit-address from anatop regulators
From: Fabio Estevam @ 2018-05-14 13:31 UTC (permalink / raw)
  To: shawnguo; +Cc: Fabio Estevam, devicetree, robh+dt, linux-arm-kernel

From: Fabio Estevam <fabio.estevam@nxp.com>

Remove unit-address and reg property from anatop regulators to fix
the following DTC warnings with W=1:

arch/arm/boot/dts/imx6dl-apf6dev.dtb: Warning (unique_unit_address): /soc/aips-bus@2000000/anatop@20c8000/regulator-vddcore@20c8140: duplicate unit-address (also used in node /soc/aips-bus@2000000/anatop@20c8000/regulator-vddpu@20c8140)
arch/arm/boot/dts/imx6dl-apf6dev.dtb: Warning (unique_unit_address): /soc/aips-bus@2000000/anatop@20c8000/regulator-vddcore@20c8140: duplicate unit-address (also used in node /soc/aips-bus@2000000/anatop@20c8000/regulator-vddsoc@20c8140)
arch/arm/boot/dts/imx6dl-apf6dev.dtb: Warning (unique_unit_address): /soc/aips-bus@2000000/anatop@20c8000/regulator-vddpu@20c8140: duplicate unit-address (also used in node /soc/aips-bus@2000000/anatop@20c8000/regulator-vddsoc@20c8140)

Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
---
Changes since v1:
- Send it is a standalone patch instead of a patch series.

 arch/arm/boot/dts/imx6qdl.dtsi | 20 ++++++--------------
 arch/arm/boot/dts/imx6sl.dtsi  | 20 ++++++--------------
 arch/arm/boot/dts/imx6sx.dtsi  | 20 ++++++--------------
 arch/arm/boot/dts/imx6ul.dtsi  | 11 +++--------
 arch/arm/boot/dts/imx7s.dtsi   |  8 ++------
 5 files changed, 23 insertions(+), 56 deletions(-)

diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
index 69648e2..22942dd 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -692,11 +692,8 @@
 				interrupts = <0 49 IRQ_TYPE_LEVEL_HIGH>,
 					     <0 54 IRQ_TYPE_LEVEL_HIGH>,
 					     <0 127 IRQ_TYPE_LEVEL_HIGH>;
-				#address-cells = <1>;
-				#size-cells = <0>;
 
-				regulator-1p1@20c8110 {
-					reg = <0x20c8110>;
+				regulator-1p1 {
 					compatible = "fsl,anatop-regulator";
 					regulator-name = "vdd1p1";
 					regulator-min-microvolt = <1000000>;
@@ -711,8 +708,7 @@
 					anatop-enable-bit = <0>;
 				};
 
-				regulator-3p0@20c8120 {
-					reg = <0x20c8120>;
+				regulator-3p0 {
 					compatible = "fsl,anatop-regulator";
 					regulator-name = "vdd3p0";
 					regulator-min-microvolt = <2800000>;
@@ -727,8 +723,7 @@
 					anatop-enable-bit = <0>;
 				};
 
-				regulator-2p5@20c8130 {
-					reg = <0x20c8130>;
+				regulator-2p5 {
 					compatible = "fsl,anatop-regulator";
 					regulator-name = "vdd2p5";
 					regulator-min-microvolt = <2250000>;
@@ -743,8 +738,7 @@
 					anatop-enable-bit = <0>;
 				};
 
-				reg_arm: regulator-vddcore@20c8140 {
-					reg = <0x20c8140>;
+				reg_arm: regulator-vddcore {
 					compatible = "fsl,anatop-regulator";
 					regulator-name = "vddarm";
 					regulator-min-microvolt = <725000>;
@@ -761,8 +755,7 @@
 					anatop-max-voltage = <1450000>;
 				};
 
-				reg_pu: regulator-vddpu@20c8140 {
-					reg = <0x20c8140>;
+				reg_pu: regulator-vddpu {
 					compatible = "fsl,anatop-regulator";
 					regulator-name = "vddpu";
 					regulator-min-microvolt = <725000>;
@@ -779,8 +772,7 @@
 					anatop-max-voltage = <1450000>;
 				};
 
-				reg_soc: regulator-vddsoc@20c8140 {
-					reg = <0x20c8140>;
+				reg_soc: regulator-vddsoc {
 					compatible = "fsl,anatop-regulator";
 					regulator-name = "vddsoc";
 					regulator-min-microvolt = <725000>;
diff --git a/arch/arm/boot/dts/imx6sl.dtsi b/arch/arm/boot/dts/imx6sl.dtsi
index 2002db2..8c838ba 100644
--- a/arch/arm/boot/dts/imx6sl.dtsi
+++ b/arch/arm/boot/dts/imx6sl.dtsi
@@ -524,11 +524,8 @@
 				interrupts = <0 49 IRQ_TYPE_LEVEL_HIGH>,
 					     <0 54 IRQ_TYPE_LEVEL_HIGH>,
 					     <0 127 IRQ_TYPE_LEVEL_HIGH>;
-				#address-cells = <1>;
-				#size-cells = <0>;
 
-				regulator-1p1@20c8110 {
-					reg = <0x20c8110>;
+				regulator-1p1 {
 					compatible = "fsl,anatop-regulator";
 					regulator-name = "vdd1p1";
 					regulator-min-microvolt = <800000>;
@@ -543,8 +540,7 @@
 					anatop-enable-bit = <0>;
 				};
 
-				regulator-3p0@20c8120 {
-					reg = <0x20c8120>;
+				regulator-3p0 {
 					compatible = "fsl,anatop-regulator";
 					regulator-name = "vdd3p0";
 					regulator-min-microvolt = <2800000>;
@@ -559,8 +555,7 @@
 					anatop-enable-bit = <0>;
 				};
 
-				regulator-2p5@20c8130 {
-					reg = <0x20c8130>;
+				regulator-2p5 {
 					compatible = "fsl,anatop-regulator";
 					regulator-name = "vdd2p5";
 					regulator-min-microvolt = <2100000>;
@@ -575,8 +570,7 @@
 					anatop-enable-bit = <0>;
 				};
 
-				reg_arm: regulator-vddcore@20c8140 {
-					reg = <0x20c8140>;
+				reg_arm: regulator-vddcore {
 					compatible = "fsl,anatop-regulator";
 					regulator-name = "vddarm";
 					regulator-min-microvolt = <725000>;
@@ -593,8 +587,7 @@
 					anatop-max-voltage = <1450000>;
 				};
 
-				reg_pu: regulator-vddpu@20c8140 {
-					reg = <0x20c8140>;
+				reg_pu: regulator-vddpu {
 					compatible = "fsl,anatop-regulator";
 					regulator-name = "vddpu";
 					regulator-min-microvolt = <725000>;
@@ -611,8 +604,7 @@
 					anatop-max-voltage = <1450000>;
 				};
 
-				reg_soc: regulator-vddsoc@20c8140 {
-					reg = <0x20c8140>;
+				reg_soc: regulator-vddsoc {
 					compatible = "fsl,anatop-regulator";
 					regulator-name = "vddsoc";
 					regulator-min-microvolt = <725000>;
diff --git a/arch/arm/boot/dts/imx6sx.dtsi b/arch/arm/boot/dts/imx6sx.dtsi
index 7e463d2..4a97513 100644
--- a/arch/arm/boot/dts/imx6sx.dtsi
+++ b/arch/arm/boot/dts/imx6sx.dtsi
@@ -591,11 +591,8 @@
 				interrupts = <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>,
 					     <GIC_SPI 54 IRQ_TYPE_LEVEL_HIGH>,
 					     <GIC_SPI 127 IRQ_TYPE_LEVEL_HIGH>;
-				#address-cells = <1>;
-				#size-cells = <0>;
 
-				regulator-1p1@20c8110 {
-					reg = <0x20c8110>;
+				regulator-1p1 {
 					compatible = "fsl,anatop-regulator";
 					regulator-name = "vdd1p1";
 					regulator-min-microvolt = <800000>;
@@ -610,8 +607,7 @@
 					anatop-enable-bit = <0>;
 				};
 
-				regulator-3p0@20c8120 {
-					reg = <0x20c8120>;
+				regulator-3p0 {
 					compatible = "fsl,anatop-regulator";
 					regulator-name = "vdd3p0";
 					regulator-min-microvolt = <2800000>;
@@ -626,8 +622,7 @@
 					anatop-enable-bit = <0>;
 				};
 
-				regulator-2p5@20c8130 {
-					reg = <0x20c8130>;
+				regulator-2p5 {
 					compatible = "fsl,anatop-regulator";
 					regulator-name = "vdd2p5";
 					regulator-min-microvolt = <2100000>;
@@ -642,8 +637,7 @@
 					anatop-enable-bit = <0>;
 				};
 
-				reg_arm: regulator-vddcore@20c8140 {
-					reg = <0x20c8140>;
+				reg_arm: regulator-vddcore {
 					compatible = "fsl,anatop-regulator";
 					regulator-name = "vddarm";
 					regulator-min-microvolt = <725000>;
@@ -660,8 +654,7 @@
 					anatop-max-voltage = <1450000>;
 				};
 
-				reg_pcie: regulator-vddpcie@20c8140 {
-					reg = <0x20c8140>;
+				reg_pcie: regulator-vddpcie {
 					compatible = "fsl,anatop-regulator";
 					regulator-name = "vddpcie";
 					regulator-min-microvolt = <725000>;
@@ -677,8 +670,7 @@
 					anatop-max-voltage = <1450000>;
 				};
 
-				reg_soc: regulator-vddsoc@20c8140 {
-					reg = <0x20c8140>;
+				reg_soc: regulator-vddsoc {
 					compatible = "fsl,anatop-regulator";
 					regulator-name = "vddsoc";
 					regulator-min-microvolt = <725000>;
diff --git a/arch/arm/boot/dts/imx6ul.dtsi b/arch/arm/boot/dts/imx6ul.dtsi
index 2b854d1..1818b6c 100644
--- a/arch/arm/boot/dts/imx6ul.dtsi
+++ b/arch/arm/boot/dts/imx6ul.dtsi
@@ -551,11 +551,8 @@
 				interrupts = <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>,
 					     <GIC_SPI 54 IRQ_TYPE_LEVEL_HIGH>,
 					     <GIC_SPI 127 IRQ_TYPE_LEVEL_HIGH>;
-				#address-cells = <1>;
-				#size-cells = <0>;
 
-				reg_3p0: regulator-3p0@20c8110 {
-					reg = <0x20c8110>;
+				reg_3p0: regulator-3p0 {
 					compatible = "fsl,anatop-regulator";
 					regulator-name = "vdd3p0";
 					regulator-min-microvolt = <2625000>;
@@ -569,8 +566,7 @@
 					anatop-enable-bit = <0>;
 				};
 
-				reg_arm: regulator-vddcore@20c8140 {
-					reg = <0x20c8140>;
+				reg_arm: regulator-vddcore {
 					compatible = "fsl,anatop-regulator";
 					regulator-name = "cpu";
 					regulator-min-microvolt = <725000>;
@@ -587,8 +583,7 @@
 					anatop-max-voltage = <1450000>;
 				};
 
-				reg_soc: regulator-vddsoc@20c8140 {
-					reg = <0x20c8140>;
+				reg_soc: regulator-vddsoc {
 					compatible = "fsl,anatop-regulator";
 					regulator-name = "vddsoc";
 					regulator-min-microvolt = <725000>;
diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
index b416d2b..99f92ec 100644
--- a/arch/arm/boot/dts/imx7s.dtsi
+++ b/arch/arm/boot/dts/imx7s.dtsi
@@ -557,11 +557,8 @@
 				reg = <0x30360000 0x10000>;
 				interrupts = <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>,
 					<GIC_SPI 51 IRQ_TYPE_LEVEL_HIGH>;
-				#address-cells = <1>;
-				#size-cells = <0>;
 
-				reg_1p0d: regulator-vdd1p0d@30360210 {
-					reg = <0x30360210>;
+				reg_1p0d: regulator-vdd1p0d {
 					compatible = "fsl,anatop-regulator";
 					regulator-name = "vdd1p0d";
 					regulator-min-microvolt = <800000>;
@@ -575,8 +572,7 @@
 					anatop-enable-bit = <0>;
 				};
 
-				reg_1p2: regulator-vdd1p2@30360220 {
-					reg = <0x30360220>;
+				reg_1p2: regulator-vdd1p2 {
 					compatible = "fsl,anatop-regulator";
 					regulator-name = "vdd1p2";
 					regulator-min-microvolt = <1100000>;
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH v2 09/11] docs: Fix some broken references
From: Masami Hiramatsu @ 2018-05-14 13:31 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Catalin Marinas, Will Deacon, dri-devel, Jaroslav Kysela,
	Eric Paris, linux-clk, James Morris, Alan Stern, xen-devel,
	Boqun Feng, Nicholas Piggin, Thomas Gleixner, Antoine Jacquet,
	Greg Kroah-Hartman, linux-usb, linux-kernel, Li Zefan,
	linux-crypto, Mark Rutland, alsa-devel, Linux Doc Mailing List,
	David Airlie, Max Filippov, Harry Wei, selinux, Paul
In-Reply-To: <e959f23d6f6905ee606fadfda13e2bb37deed017.1525870886.git.mchehab+samsung@kernel.org>

On Wed,  9 May 2018 10:18:52 -0300
Mauro Carvalho Chehab <mchehab+samsung@kernel.org> wrote:

> As we move stuff around, some doc references are broken. Fix some of
> them via this script:
> 	./scripts/documentation-file-ref-check --fix-rst
> 
> Manually checked if the produced result is valid, removing a few
> false-positives.

The part of kprobes and trace_kprobe changes look good to me :)

Acked-by: Masami Hiramatsu <mhiramat@kernel.org>

Thank you,

> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
> ---
>  .../admin-guide/kernel-parameters.txt         |  4 ++--
>  .../bindings/input/rotary-encoder.txt         |  2 +-
>  Documentation/driver-api/gpio/consumer.rst    |  2 +-
>  Documentation/kprobes.txt                     |  4 ++--
>  Documentation/trace/coresight.txt             |  2 +-
>  Documentation/trace/events.rst                |  2 +-
>  Documentation/trace/ftrace-uses.rst           |  2 +-
>  Documentation/trace/histogram.txt             |  2 +-
>  Documentation/trace/intel_th.rst              |  2 +-
>  Documentation/trace/tracepoint-analysis.rst   |  6 +++---
>  Documentation/translations/ja_JP/howto.rst    |  4 ++--
>  .../translations/zh_CN/magic-number.txt       |  4 ++--
>  .../zh_CN/video4linux/omap3isp.txt            |  4 ++--
>  MAINTAINERS                                   | 20 +++++++++----------
>  arch/Kconfig                                  |  2 +-
>  arch/arm/include/asm/cacheflush.h             |  2 +-
>  arch/arm64/include/asm/cacheflush.h           |  2 +-
>  arch/microblaze/include/asm/cacheflush.h      |  2 +-
>  arch/um/Kconfig.um                            |  2 +-
>  arch/unicore32/include/asm/cacheflush.h       |  2 +-
>  arch/x86/entry/vsyscall/vsyscall_64.c         |  2 +-
>  arch/xtensa/include/asm/cacheflush.h          |  4 ++--
>  block/Kconfig                                 |  2 +-
>  certs/Kconfig                                 |  2 +-
>  crypto/asymmetric_keys/asymmetric_type.c      |  2 +-
>  crypto/asymmetric_keys/signature.c            |  2 +-
>  drivers/char/Kconfig                          |  2 +-
>  drivers/clk/clk.c                             |  4 ++--
>  drivers/clk/ingenic/cgu.h                     |  2 +-
>  drivers/gpu/vga/Kconfig                       |  2 +-
>  drivers/gpu/vga/vgaarb.c                      |  2 +-
>  drivers/input/joystick/Kconfig                | 10 +++++-----
>  drivers/input/joystick/iforce/Kconfig         |  2 +-
>  drivers/input/joystick/walkera0701.c          |  2 +-
>  drivers/input/misc/Kconfig                    |  4 ++--
>  drivers/input/misc/rotary_encoder.c           |  2 +-
>  drivers/input/mouse/Kconfig                   |  6 +++---
>  drivers/input/mouse/alps.c                    |  2 +-
>  drivers/input/touchscreen/wm97xx-core.c       |  2 +-
>  drivers/lightnvm/pblk-rb.c                    |  2 +-
>  drivers/md/bcache/Kconfig                     |  2 +-
>  drivers/md/bcache/btree.c                     |  2 +-
>  drivers/md/bcache/extents.c                   |  2 +-
>  drivers/media/dvb-core/dvb_ringbuffer.c       |  2 +-
>  drivers/media/pci/meye/Kconfig                |  2 +-
>  drivers/media/platform/pxa_camera.c           |  4 ++--
>  .../soc_camera/sh_mobile_ceu_camera.c         |  2 +-
>  drivers/media/radio/Kconfig                   |  2 +-
>  drivers/media/radio/si470x/Kconfig            |  2 +-
>  drivers/media/usb/dvb-usb-v2/lmedm04.c        |  2 +-
>  drivers/media/usb/zr364xx/Kconfig             |  2 +-
>  drivers/parport/Kconfig                       |  6 +++---
>  drivers/staging/media/bcm2048/TODO            |  2 +-
>  include/keys/asymmetric-subtype.h             |  2 +-
>  include/keys/asymmetric-type.h                |  2 +-
>  include/linux/assoc_array.h                   |  2 +-
>  include/linux/assoc_array_priv.h              |  2 +-
>  include/linux/circ_buf.h                      |  2 +-
>  include/linux/ftrace.h                        |  2 +-
>  include/linux/rculist_nulls.h                 |  2 +-
>  include/uapi/linux/prctl.h                    |  2 +-
>  include/xen/interface/io/kbdif.h              |  2 +-
>  kernel/cgroup/cpuset.c                        |  2 +-
>  kernel/trace/Kconfig                          | 16 +++++++--------
>  lib/Kconfig                                   |  2 +-
>  security/selinux/hooks.c                      |  2 +-
>  sound/core/Kconfig                            |  4 ++--
>  sound/drivers/Kconfig                         |  4 ++--
>  sound/pci/Kconfig                             | 10 +++++-----
>  tools/include/uapi/linux/prctl.h              |  2 +-
>  tools/lib/api/fs/fs.c                         |  2 +-
>  tools/memory-model/README                     | 10 +++++-----
>  tools/perf/util/bpf-prologue.c                |  2 +-
>  .../config/custom-timeline-functions.cfg      |  4 ++--
>  74 files changed, 120 insertions(+), 120 deletions(-)
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply

* Re: [PATCH] ARM: dts: at91-sama5d2_xplained: Use IRQ_TYPE specifier
From: Alexandre Belloni @ 2018-05-14 13:30 UTC (permalink / raw)
  To: Hernán Gonzalez
  Cc: nicolas.ferre, robh+dt, mark.rutland, linux-arm-kernel,
	devicetree, linux-kernel
In-Reply-To: <1526253724-3596-1-git-send-email-hernan@vanguardiasur.com.ar>

On 13/05/2018 20:22:04-0300, Hernán Gonzalez wrote:
> GPIO_ACTIVE_LOW was being used to specify an interrupt, use
> IRQ_TYPE_EDGE_RISING instead. This improves DT readability.
> 
> Signed-off-by: Hernán Gonzalez <hernan@vanguardiasur.com.ar>
> ---
>  arch/arm/boot/dts/at91-sama5d2_xplained.dts | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
Applied, thanks.

-- 
Alexandre Belloni, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply

* Re: [PATCH] dt-bindings: rtc-2123: Make the node name generic
From: Alexandre Belloni @ 2018-05-14 13:28 UTC (permalink / raw)
  To: Fabio Estevam; +Cc: robh+dt, linux-rtc, devicetree, Fabio Estevam
In-Reply-To: <1525489831-23871-1-git-send-email-festevam@gmail.com>

On 05/05/2018 00:10:31-0300, Fabio Estevam wrote:
> From: Fabio Estevam <fabio.estevam@nxp.com>
> 
> According to Devicetree Specification v0.2 document:
> 
> "The name of a node should be somewhat generic, reflecting the function
> of the device and not its precise programming model."
> 
> Do as suggested in the binding example.
> 
> Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
> ---
>  Documentation/devicetree/bindings/rtc/nxp,rtc-2123.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
Applied, thanks.

-- 
Alexandre Belloni, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply

* Re: [PATCH/RFC v3 1/4] base: devcon: add a new API to find the graph
From: Heikki Krogerus @ 2018-05-14 13:27 UTC (permalink / raw)
  To: Yoshihiro Shimoda
  Cc: gregkh, robh+dt, mark.rutland, hdegoede, andy.shevchenko,
	linux-usb, linux-renesas-soc, devicetree
In-Reply-To: <1526289360-3997-2-git-send-email-yoshihiro.shimoda.uh@renesas.com>

On Mon, May 14, 2018 at 06:15:57PM +0900, Yoshihiro Shimoda wrote:
> This patch adds a new API "device_connection_find_by_graph()" to
> find device connection by using graph.
> 
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> ---
>  Documentation/driver-api/device_connection.rst |  4 +--
>  drivers/base/devcon.c                          | 43 ++++++++++++++++++++++++++
>  include/linux/device.h                         |  2 ++
>  3 files changed, 47 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/driver-api/device_connection.rst b/Documentation/driver-api/device_connection.rst
> index affbc556..2e2d26f 100644
> --- a/Documentation/driver-api/device_connection.rst
> +++ b/Documentation/driver-api/device_connection.rst
> @@ -19,7 +19,7 @@ Device connections alone do not create a dependency between the two devices.
>  They are only descriptions which are not tied to either of the devices directly.
>  A dependency between the two devices exists only if one of the two endpoint
>  devices requests a reference to the other. The descriptions themselves can be
> -defined in firmware (not yet supported) or they can be built-in.
> +defined in firmware or they can be built-in.
>  
>  Usage
>  -----
> @@ -40,4 +40,4 @@ API
>  ---
>  
>  .. kernel-doc:: drivers/base/devcon.c
> -   : functions: device_connection_find_match device_connection_find device_connection_add device_connection_remove
> +   : functions: device_connection_find_match device_connection_find device_connection_add device_connection_remove device_connection_find_by_graph
> diff --git a/drivers/base/devcon.c b/drivers/base/devcon.c
> index d427e80..5a0da33 100644
> --- a/drivers/base/devcon.c
> +++ b/drivers/base/devcon.c
> @@ -7,6 +7,7 @@
>   */
>  
>  #include <linux/device.h>
> +#include <linux/property.h>
>  
>  static DEFINE_MUTEX(devcon_lock);
>  static LIST_HEAD(devcon_list);
> @@ -134,3 +135,45 @@ void device_connection_remove(struct device_connection *con)
>  	mutex_unlock(&devcon_lock);
>  }
>  EXPORT_SYMBOL_GPL(device_connection_remove);
> +
> +static int generic_graph_match(struct device *dev, void *fwnode)
> +{
> +	return dev->fwnode == fwnode;
> +}
> +
> +/**
> + * device_connection_find_by_graph - Find two devices connected together
> + * @dev: Device to find connected device
> + * @port: identifier of the @dev port node
> + * @endpoint: identifier of the @dev endpoint node
> + *
> + * Find a connection with @port and @endpoint by using graph between @dev and
> + * another device. On success returns handle to the device that is connected
> + * to @dev, with the reference count for the found device incremented. Returns
> + * NULL if no matching connection was found, or ERR_PTR(-EPROBE_DEFER) when
> + * a connection was found but the other device has not been enumerated yet.
> + */
> +struct device *device_connection_find_by_graph(struct device *dev, u32 port,
> +					       u32 endpoint)
> +{
> +	struct bus_type *bus;
> +	struct fwnode_handle *remote;
> +	struct device *conn;
> +
> +	remote = fwnode_graph_get_remote_node(dev_fwnode(dev), port, endpoint);
> +	if (!remote)
> +		return NULL;
> +
> +	for (bus = generic_match_buses[0]; bus; bus++) {
> +		conn = bus_find_device(bus, NULL, remote, generic_graph_match);
> +		if (conn)
> +			return conn;
> +	}
> +
> +	/*
> +	 * We only get called if a connection was found, tell the caller to
> +	 * wait for the other device to show up.
> +	 */
> +	return ERR_PTR(-EPROBE_DEFER);
> +}
> +EXPORT_SYMBOL_GPL(device_connection_find_by_graph);

Why do we need more API for walking through the graph?

I'm not sure exactly sure what is going on here, I'll try to study
your patches more when I have time, but the approach looks wrong. That
function looks like a helper, but just not that useful one.

We really should be able to use the existing functions. In practice
device_connection_find_match() should eventually parse the graph, then
fallback to build-in connections if no graph is found. Otherwise
parsing graph here is not really useful at all.


Thanks,

-- 
heikki

^ permalink raw reply

* Re: [PATCH 00/12] introduce support for early platform drivers
From: Rob Herring @ 2018-05-14 13:20 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Sekhar Nori, Kevin Hilman, David Lechner, Michael Turquette,
	Stephen Boyd, Arnd Bergmann, Greg Kroah-Hartman, Mark Rutland,
	Yoshinori Sato, Rich Felker, Andy Shevchenko, Marc Zyngier,
	Rafael J . Wysocki, Peter Rosin, Jiri Slaby, Thomas Gleixner,
	Daniel Lezcano, Geert Uytterhoeven, Magnus Damm, Johan Hovold
In-Reply-To: <CAMRc=McDNYQ5uk8EyRkDf8eh9q7tyF=t7PcUd6DBLqXcseJ3AQ@mail.gmail.com>

On Mon, May 14, 2018 at 6:38 AM, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> 2018-05-11 22:13 GMT+02:00 Rob Herring <robh+dt@kernel.org>:
>> On Fri, May 11, 2018 at 11:20 AM, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>>> This series is a follow-up to the RFC[1] posted a couple days ago.
>>>
>>> NOTE: this series applies on top of my recent patches[2] that move the previous
>>> implementation of early platform devices to arch/sh.
>>>
>>> Problem:
>>>
>>> Certain class of devices, such as timers, certain clock drivers and irq chip
>>> drivers need to be probed early in the boot sequence. The currently preferred
>>> approach is using one of the OF_DECLARE() macros. This however does not create
>>> a platform device which has many drawbacks - such as not being able to use
>>> devres routines, dev_ log functions or no way of deferring the init OF function
>>> if some other resources are missing.
>>
>> I skimmed though this and it doesn't look horrible (how's that for
>> positive feedback? ;) ). But before going into the details, I think
>> first there needs to be agreement this is the right direction.
>>
>> The question does remain though as to whether this class of devices
>> should be platform drivers. They can't be modules. They can't be
>> hotplugged. Can they be runtime-pm enabled? So the advantage is ...
>>
>
> The main (but not the only) advantage for drivers that can both be
> platform drivers and OF_DECLARE drivers is that we get a single entry
> point and can reuse code without resorting to checking if (!dev). It
> results in more consistent code base. Another big advantage is
> consolidation of device tree and machine code for SoC drivers used in
> different boards of which some are still using board files and others
> are defined in DT (see: DaVinci).
>
>> I assume that the clock maintainers had some reason to move clocks to
>> be platform drivers. It's just not clear to me what that was.
>>
>>> For drivers that use both platform drivers and OF_DECLARE the situation is even
>>> more complicated as the code needs to take into account that there can possibly
>>> be no struct device present. For a specific use case that we're having problems
>>> with, please refer to the recent DaVinci common-clock conversion patches and
>>> the nasty workaround that this problem implies[3].
>>
>> So devm_kzalloc will work with this solution? Why did we need
>> devm_kzalloc in the first place? The clocks can never be removed and
>> cleaning up on error paths is kind of pointless. The system would be
>> hosed, right?
>>
>
> It depends - not all clocks are necessary for system to boot.

That doesn't matter. You have a single driver for all/most of the
clocks, so the driver can't be removed.

>>> We also used to have an early platform drivers implementation but they were not
>>> integrated with the linux device model at all - they merely used the same data
>>> structures. The users could not use devres, defer probe and the early devices
>>> never became actual platform devices later on.
>>>
>>> Proposed solution:
>>>
>>> This series aims at solving this problem by (re-)introducing the concept of
>>> early platform drivers and devices - this time however in a way that seamlessly
>>> integrates with the existing platform drivers and also offers device-tree
>>> support.
>>>
>>> The idea is to provide a way for users to probe devices early, while already
>>> being able to use devres, devices resources and properties and also deferred
>>> probing.
>>>
>>> New structures are introduced: the early platform driver contains the
>>> early_probe callback which has the same signature as regular platform_device
>>> probe. This callback is called early on. The user can have both the early and
>>> regular probe speficied or only one of them and they both receive the same
>>> platform device object as argument. Any device data allocated early will be
>>> carried over to the normal probe.
>>>
>>> The architecture code is responsible for calling early_platform_start() in
>>> which the early drivers will be registered and devices populated from DT.
>>
>> Can we really do this in one spot for different devices (clk, timers,
>> irq). The sequence is all very carefully crafted. Platform specific
>> hooks is another thing to consider.
>>
>
> This is why I added support for early probe deferral - so that we can
> stop caring for the order as long as the drivers are aware of other
> resources they need and we call early_platform_start() the moment the
> earliest of the early devices is needed.

Deferred probe helps for inter-device dependencies, but I am more
concerned about timing of trying to register clocksources, irqchips,
etc. What happens if we probe drivers before the infrastructure is
initialized?

Rob

^ permalink raw reply

* Re: [PATCH v2 1/3] dt: bindings: i2c-mux-pca954x: add mux-locked property
From: Peter Rosin @ 2018-05-14 13:16 UTC (permalink / raw)
  To: Bastian Stender, Wolfram Sang
  Cc: devicetree, Michael Lawnick, linux-i2c, kernel
In-Reply-To: <a4402498-f318-2753-4220-fde6b1d372a0@pengutronix.de>

On 2018-05-14 14:26, Bastian Stender wrote:
> Hi Peter,
> 
> On 05/07/2018 11:07 AM, Peter Rosin wrote:
>> On 2018-05-04 15:10, Bastian Stender wrote:
>>> On 05/04/2018 03:04 PM, Bastian Stender wrote:
>>>> Signed-off-by: Bastian Stender <bst@pengutronix.de>
>>>> ---
>>>>    .../devicetree/bindings/i2c/i2c-mux-pca954x.txt  | 16 ++++++++++++++++
>>>>    1 file changed, 16 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
>>>> index 34d91501342e..864ac91f8c1c 100644
>>>> --- a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
>>>> +++ b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
>>>> @@ -36,6 +36,22 @@ Optional Properties:
>>>>        - first cell is the pin number
>>>>        - second cell is used to specify flags.
>>>>        See also Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
>>>> +  - mux-locked: If present, explicitly allow unrelated I2C transactions on the
>>>> +    parent I2C adapter at these times:
>>>> +    + during setup of the multiplexer
>>>> +    + between setup of the multiplexer and the child bus I2C transaction
>>>> +    + between the child bus I2C transaction and releasing of the multiplexer
>>>> +    + during releasing of the multiplexer
>>>> +
>>>> +    However, I2C transactions to devices behind all I2C multiplexers connected
>>>> +    to the same parent adapter that this multiplexer is connected to are blocked
>>>> +    for the full duration of the complete multiplexed I2C transaction (i.e.
>>>> +    including the times covered by the above list).
>>>> +    If mux-locked is not present, the multiplexer is assumed to be parent-locked.
>>>> +    This means that no unrelated I2C transactions are allowed on the parent I2C
>>>> +    adapter for the complete multiplexed I2C transaction.
>>>> +    The properties of mux-locked and parent-locked multiplexers are discussed
>>>> +    in more detail in Documentation/i2c/i2c-topology.
>>>
>>> I am not sure about this. I think it will act like the gpmux driver here
>>> so I copied it from
>>> Documentation/devicetree/bindings/i2c/i2c-mux-gpmux.txt. Is this correct?
>>
>> I don't think it's wrong, but it might be a little bit too generic. The gpmux
>> binding cannot assume too much about the actual mux, but in this case we
>> know exactly how the mux operates. I.e. the initial 4-point list can drop
>> the points "during setup/releasing of the multiplexer" because those actions
>> happen as i2c transactions that are locking the parent adapter meaning that
>> nothing can disturb them.
>>
>> However, it would be very good to know what the actual deadlock is that this
>> mux-locked is fixing. And a description of that deadlock would fit nicely in
>> the commit message. I understood it as if you could trigger it quite easily?
>> Any chance that you could make another attempt to pinpoint it with some
>> printk-debugging or something, given that lockdep wasn't helpful?
> 
> The root cause was now identified by a colleague of mine. The following
> patch set was used along with the proposed patches:
> 
>    https://www.spinics.net/lists/linux-i2c/msg32324.html
> 
> As a result __i2c_transfer() ends up without enabled clocks. The problem
> is solved in our case by switching to the mainline solution:
> 
>    https://www.spinics.net/lists/linux-i2c/msg33890.html

Nice, thanks for keeping me in the loop!

> The question remains whether the proposed patch (along with the
> suggested modifications) should be applied nonetheless?

Maybe? I will not block it because there certainly are cases where it is
needed, but someone<tm> needs to make the adjustments.

Cheers,
Peter

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox