* [PATCH v16 06/11] pwm: imx27: Use 64-bit division macro and function
From: Guru Das Srinagesh @ 2020-06-02 22:31 UTC (permalink / raw)
To: linux-pwm, Thierry Reding, Uwe Kleine-König
Cc: linux-arm-kernel, Guru Das Srinagesh, Daniel Thompson,
Fabio Estevam, Arnd Bergmann, David Collins, Stephen Boyd,
Shawn Guo, Sascha Hauer, linux-kernel, Geert Uytterhoeven,
Dan Carpenter, Pengutronix Kernel Team, Joe Perches,
Subbaraman Narayanamurthy, Lee Jones, Guenter Roeck,
NXP Linux Team
In-Reply-To: <cover.1591136989.git.gurus@codeaurora.org>
Since the PWM framework is switching struct pwm_state.period's
datatype to u64, prepare for this transition by using
DIV_ROUND_UP_ULL to handle a 64-bit dividend, and div64_u64 to handle a
64-bit divisor.
Also handle a possible overflow in the calculation of period_cycles when
both clk_rate and period are large numbers. This logic was unit-tested
out by calculating period_cycles using both the existing logic and the
proposed one, and the results are as below.
----------------------------------------------------------------------------
clk_rate period existing proposed
----------------------------------------------------------------------------
1000000000 18446744073709551615 18446744072 18446744073000000000
(U64_MAX)
----------------------------------------------------------------------------
1000000000 4294967291 4294967291 4294967291
----------------------------------------------------------------------------
Overflow occurs in the first case with the existing logic, whereas the
proposed logic handles it better, sacrificing some precision in a best-effort
attempt to handle overflow. As for the second case where there are
more typical values of period, the proposed logic handles that correctly
too.
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: NXP Linux Team <linux-imx@nxp.com>
Signed-off-by: Guru Das Srinagesh <gurus@codeaurora.org>
---
drivers/pwm/pwm-imx27.c | 48 +++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 43 insertions(+), 5 deletions(-)
diff --git a/drivers/pwm/pwm-imx27.c b/drivers/pwm/pwm-imx27.c
index 732a6f3..147729d 100644
--- a/drivers/pwm/pwm-imx27.c
+++ b/drivers/pwm/pwm-imx27.c
@@ -202,7 +202,7 @@ static void pwm_imx27_wait_fifo_slot(struct pwm_chip *chip,
sr = readl(imx->mmio_base + MX3_PWMSR);
fifoav = FIELD_GET(MX3_PWMSR_FIFOAV, sr);
if (fifoav == MX3_PWMSR_FIFOAV_4WORDS) {
- period_ms = DIV_ROUND_UP(pwm_get_period(pwm),
+ period_ms = DIV_ROUND_UP_ULL(pwm_get_period(pwm),
NSEC_PER_MSEC);
msleep(period_ms);
@@ -212,6 +212,45 @@ static void pwm_imx27_wait_fifo_slot(struct pwm_chip *chip,
}
}
+static int pwm_imx27_calc_period_cycles(const struct pwm_state *state,
+ unsigned long clk_rate,
+ unsigned long *period_cycles)
+{
+ u64 c = 0, c1, c2;
+
+ c1 = clk_rate;
+ c2 = state->period;
+ if (c2 > c1) {
+ c2 = c1;
+ c1 = state->period;
+ }
+
+ if (!c1 || !c2) {
+ pr_err("clk rate and period should be nonzero\n");
+ return -EINVAL;
+ }
+
+ if (c2 <= div64_u64(U64_MAX, c1)) {
+ c = c1 * c2;
+ do_div(c, 1000000000);
+ } else if (c2 <= div64_u64(U64_MAX, div64_u64(c1, 1000))) {
+ do_div(c1, 1000);
+ c = c1 * c2;
+ do_div(c, 1000000);
+ } else if (c2 <= div64_u64(U64_MAX, div64_u64(c1, 1000000))) {
+ do_div(c1, 1000000);
+ c = c1 * c2;
+ do_div(c, 1000);
+ } else if (c2 <= div64_u64(U64_MAX, div64_u64(c1, 1000000000))) {
+ do_div(c1, 1000000000);
+ c = c1 * c2;
+ }
+
+ *period_cycles = c;
+
+ return 0;
+}
+
static int pwm_imx27_apply(struct pwm_chip *chip, struct pwm_device *pwm,
const struct pwm_state *state)
{
@@ -226,10 +265,9 @@ static int pwm_imx27_apply(struct pwm_chip *chip, struct pwm_device *pwm,
pwm_get_state(pwm, &cstate);
clkrate = clk_get_rate(imx->clk_per);
- c = clkrate * state->period;
-
- do_div(c, NSEC_PER_SEC);
- period_cycles = c;
+ ret = pwm_imx27_calc_period_cycles(state, clkrate, &period_cycles);
+ if (ret)
+ return ret;
prescale = period_cycles / 0x10000 + 1;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v16 01/11] drm/i915: Use 64-bit division macro
From: Guru Das Srinagesh @ 2020-06-02 22:31 UTC (permalink / raw)
To: linux-pwm, Thierry Reding, Uwe Kleine-König
Cc: linux-arm-kernel, Guru Das Srinagesh, Daniel Thompson,
Arnd Bergmann, David Collins, Stephen Boyd, linux-kernel,
Geert Uytterhoeven, Dan Carpenter, Joe Perches,
Subbaraman Narayanamurthy, Lee Jones, Guenter Roeck
In-Reply-To: <cover.1591136989.git.gurus@codeaurora.org>
Since the PWM framework is switching struct pwm_state.duty_cycle's
datatype to u64, prepare for this transition by using DIV_ROUND_UP_ULL
to handle a 64-bit dividend.
Signed-off-by: Guru Das Srinagesh <gurus@codeaurora.org>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Acked-by: Jani Nikula <jani.nikula@intel.com>
---
drivers/gpu/drm/i915/display/intel_panel.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_panel.c b/drivers/gpu/drm/i915/display/intel_panel.c
index 276f438..81547a0 100644
--- a/drivers/gpu/drm/i915/display/intel_panel.c
+++ b/drivers/gpu/drm/i915/display/intel_panel.c
@@ -1920,7 +1920,7 @@ static int pwm_setup_backlight(struct intel_connector *connector,
return retval;
}
- level = DIV_ROUND_UP(pwm_get_duty_cycle(panel->backlight.pwm) * 100,
+ level = DIV_ROUND_UP_ULL(pwm_get_duty_cycle(panel->backlight.pwm) * 100,
CRC_PMIC_PWM_PERIOD_NS);
panel->backlight.level =
intel_panel_compute_brightness(connector, level);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v16 07/11] pwm: sifive: Use 64-bit division macro
From: Guru Das Srinagesh @ 2020-06-02 22:31 UTC (permalink / raw)
To: linux-pwm, Thierry Reding, Uwe Kleine-König
Cc: linux-arm-kernel, Guru Das Srinagesh, Daniel Thompson,
Arnd Bergmann, David Collins, Stephen Boyd, linux-kernel,
Geert Uytterhoeven, Dan Carpenter, Joe Perches,
Subbaraman Narayanamurthy, Lee Jones, Guenter Roeck
In-Reply-To: <cover.1591136989.git.gurus@codeaurora.org>
Since the PWM framework is switching struct pwm_args.period's datatype
to u64, prepare for this transition by using DIV64_U64_ROUND_CLOSEST to
handle a 64-bit divisor.
Signed-off-by: Guru Das Srinagesh <gurus@codeaurora.org>
Acked-by: Palmer Dabbelt <palmerdabbelt@google.com>
---
drivers/pwm/pwm-sifive.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pwm/pwm-sifive.c b/drivers/pwm/pwm-sifive.c
index cc63f9b..62de0bb 100644
--- a/drivers/pwm/pwm-sifive.c
+++ b/drivers/pwm/pwm-sifive.c
@@ -181,7 +181,7 @@ static int pwm_sifive_apply(struct pwm_chip *chip, struct pwm_device *pwm,
* consecutively
*/
num = (u64)duty_cycle * (1U << PWM_SIFIVE_CMPWIDTH);
- frac = DIV_ROUND_CLOSEST_ULL(num, state->period);
+ frac = DIV64_U64_ROUND_CLOSEST(num, state->period);
/* The hardware cannot generate a 100% duty cycle */
frac = min(frac, (1U << PWM_SIFIVE_CMPWIDTH) - 1);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v16 03/11] ir-rx51: Use 64-bit division macro
From: Guru Das Srinagesh @ 2020-06-02 22:31 UTC (permalink / raw)
To: linux-pwm, Thierry Reding, Uwe Kleine-König
Cc: linux-arm-kernel, Guru Das Srinagesh, Daniel Thompson,
Arnd Bergmann, David Collins, Stephen Boyd, linux-kernel,
Geert Uytterhoeven, Dan Carpenter, Joe Perches,
Subbaraman Narayanamurthy, Lee Jones, Guenter Roeck
In-Reply-To: <cover.1591136989.git.gurus@codeaurora.org>
Since the PWM framework is switching struct pwm_state.period's datatype
to u64, prepare for this transition by using DIV_ROUND_CLOSEST_ULL to
handle a 64-bit dividend.
Signed-off-by: Guru Das Srinagesh <gurus@codeaurora.org>
Acked-by: Sean Young <sean@mess.org>
---
drivers/media/rc/ir-rx51.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
index 8574eda..9a5dfd7 100644
--- a/drivers/media/rc/ir-rx51.c
+++ b/drivers/media/rc/ir-rx51.c
@@ -241,7 +241,8 @@ static int ir_rx51_probe(struct platform_device *dev)
}
/* Use default, in case userspace does not set the carrier */
- ir_rx51.freq = DIV_ROUND_CLOSEST(pwm_get_period(pwm), NSEC_PER_SEC);
+ ir_rx51.freq = DIV_ROUND_CLOSEST_ULL(pwm_get_period(pwm),
+ NSEC_PER_SEC);
pwm_put(pwm);
hrtimer_init(&ir_rx51.timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v16 09/11] backlight: pwm_bl: Use 64-bit division function
From: Guru Das Srinagesh @ 2020-06-02 22:31 UTC (permalink / raw)
To: linux-pwm, Thierry Reding, Uwe Kleine-König
Cc: linux-arm-kernel, Guru Das Srinagesh, Daniel Thompson,
Arnd Bergmann, David Collins, Stephen Boyd, linux-kernel,
Geert Uytterhoeven, Dan Carpenter, Joe Perches,
Subbaraman Narayanamurthy, Lee Jones, Guenter Roeck
In-Reply-To: <cover.1591136989.git.gurus@codeaurora.org>
Since the PWM framework is switching struct pwm_state.period's datatype
to u64, prepare for this transition by using div_u64 to handle a 64-bit
dividend instead of a straight division operation.
Signed-off-by: Guru Das Srinagesh <gurus@codeaurora.org>
Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
Acked-by: Lee Jones <lee.jones@linaro.org>
---
drivers/video/backlight/pwm_bl.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index 82b8d75..464baad 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -606,7 +606,8 @@ static int pwm_backlight_probe(struct platform_device *pdev)
pb->scale = data->max_brightness;
}
- pb->lth_brightness = data->lth_brightness * (state.period / pb->scale);
+ pb->lth_brightness = data->lth_brightness * (div_u64(state.period,
+ pb->scale));
props.type = BACKLIGHT_RAW;
props.max_brightness = data->max_brightness;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v16 11/11] pwm: core: Convert period and duty cycle to u64
From: Guru Das Srinagesh @ 2020-06-02 22:31 UTC (permalink / raw)
To: linux-pwm, Thierry Reding, Uwe Kleine-König
Cc: linux-arm-kernel, Guru Das Srinagesh, Daniel Thompson,
Arnd Bergmann, David Collins, Stephen Boyd, linux-kernel,
Geert Uytterhoeven, Dan Carpenter, Joe Perches,
Subbaraman Narayanamurthy, Lee Jones, Guenter Roeck
In-Reply-To: <cover.1591136989.git.gurus@codeaurora.org>
Because period and duty cycle are defined as ints with units of
nanoseconds, the maximum time duration that can be set is limited to
~2.147 seconds. Change their definitions to u64 in the structs of the
PWM framework so that higher durations may be set.
Also use the right format specifiers in debug prints in both core.c as
well as pwm-stm32-lp.c.
Reported-by: kbuild test robot <lkp@intel.com>
Signed-off-by: Guru Das Srinagesh <gurus@codeaurora.org>
Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/pwm/core.c | 14 +++++++-------
drivers/pwm/pwm-stm32-lp.c | 2 +-
drivers/pwm/sysfs.c | 8 ++++----
include/linux/pwm.h | 12 ++++++------
4 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index bca0496..a2ff6dd 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -510,12 +510,12 @@ static void pwm_apply_state_debug(struct pwm_device *pwm,
last->period > s2.period &&
last->period <= state->period)
dev_warn(chip->dev,
- ".apply didn't pick the best available period (requested: %u, applied: %u, possible: %u)\n",
+ ".apply didn't pick the best available period (requested: %llu, applied: %llu, possible: %llu)\n",
state->period, s2.period, last->period);
if (state->enabled && state->period < s2.period)
dev_warn(chip->dev,
- ".apply is supposed to round down period (requested: %u, applied: %u)\n",
+ ".apply is supposed to round down period (requested: %llu, applied: %llu)\n",
state->period, s2.period);
if (state->enabled &&
@@ -524,14 +524,14 @@ static void pwm_apply_state_debug(struct pwm_device *pwm,
last->duty_cycle > s2.duty_cycle &&
last->duty_cycle <= state->duty_cycle)
dev_warn(chip->dev,
- ".apply didn't pick the best available duty cycle (requested: %u/%u, applied: %u/%u, possible: %u/%u)\n",
+ ".apply didn't pick the best available duty cycle (requested: %llu/%llu, applied: %llu/%llu, possible: %llu/%llu)\n",
state->duty_cycle, state->period,
s2.duty_cycle, s2.period,
last->duty_cycle, last->period);
if (state->enabled && state->duty_cycle < s2.duty_cycle)
dev_warn(chip->dev,
- ".apply is supposed to round down duty_cycle (requested: %u/%u, applied: %u/%u)\n",
+ ".apply is supposed to round down duty_cycle (requested: %llu/%llu, applied: %llu/%llu)\n",
state->duty_cycle, state->period,
s2.duty_cycle, s2.period);
@@ -558,7 +558,7 @@ static void pwm_apply_state_debug(struct pwm_device *pwm,
(s1.enabled && s1.period != last->period) ||
(s1.enabled && s1.duty_cycle != last->duty_cycle)) {
dev_err(chip->dev,
- ".apply is not idempotent (ena=%d pol=%d %u/%u) -> (ena=%d pol=%d %u/%u)\n",
+ ".apply is not idempotent (ena=%d pol=%d %llu/%llu) -> (ena=%d pol=%d %llu/%llu)\n",
s1.enabled, s1.polarity, s1.duty_cycle, s1.period,
last->enabled, last->polarity, last->duty_cycle,
last->period);
@@ -1284,8 +1284,8 @@ static void pwm_dbg_show(struct pwm_chip *chip, struct seq_file *s)
if (state.enabled)
seq_puts(s, " enabled");
- seq_printf(s, " period: %u ns", state.period);
- seq_printf(s, " duty: %u ns", state.duty_cycle);
+ seq_printf(s, " period: %llu ns", state.period);
+ seq_printf(s, " duty: %llu ns", state.duty_cycle);
seq_printf(s, " polarity: %s",
state.polarity ? "inverse" : "normal");
diff --git a/drivers/pwm/pwm-stm32-lp.c b/drivers/pwm/pwm-stm32-lp.c
index 67fca62..134c146 100644
--- a/drivers/pwm/pwm-stm32-lp.c
+++ b/drivers/pwm/pwm-stm32-lp.c
@@ -61,7 +61,7 @@ static int stm32_pwm_lp_apply(struct pwm_chip *chip, struct pwm_device *pwm,
do_div(div, NSEC_PER_SEC);
if (!div) {
/* Clock is too slow to achieve requested period. */
- dev_dbg(priv->chip.dev, "Can't reach %u ns\n", state->period);
+ dev_dbg(priv->chip.dev, "Can't reach %llu ns\n", state->period);
return -EINVAL;
}
diff --git a/drivers/pwm/sysfs.c b/drivers/pwm/sysfs.c
index 2389b86..449dbc0 100644
--- a/drivers/pwm/sysfs.c
+++ b/drivers/pwm/sysfs.c
@@ -42,7 +42,7 @@ static ssize_t period_show(struct device *child,
pwm_get_state(pwm, &state);
- return sprintf(buf, "%u\n", state.period);
+ return sprintf(buf, "%llu\n", state.period);
}
static ssize_t period_store(struct device *child,
@@ -52,10 +52,10 @@ static ssize_t period_store(struct device *child,
struct pwm_export *export = child_to_pwm_export(child);
struct pwm_device *pwm = export->pwm;
struct pwm_state state;
- unsigned int val;
+ u64 val;
int ret;
- ret = kstrtouint(buf, 0, &val);
+ ret = kstrtou64(buf, 0, &val);
if (ret)
return ret;
@@ -77,7 +77,7 @@ static ssize_t duty_cycle_show(struct device *child,
pwm_get_state(pwm, &state);
- return sprintf(buf, "%u\n", state.duty_cycle);
+ return sprintf(buf, "%llu\n", state.duty_cycle);
}
static ssize_t duty_cycle_store(struct device *child,
diff --git a/include/linux/pwm.h b/include/linux/pwm.h
index 2635b2a..a13ff38 100644
--- a/include/linux/pwm.h
+++ b/include/linux/pwm.h
@@ -39,7 +39,7 @@ enum pwm_polarity {
* current PWM hardware state.
*/
struct pwm_args {
- unsigned int period;
+ u64 period;
enum pwm_polarity polarity;
};
@@ -56,8 +56,8 @@ enum {
* @enabled: PWM enabled status
*/
struct pwm_state {
- unsigned int period;
- unsigned int duty_cycle;
+ u64 period;
+ u64 duty_cycle;
enum pwm_polarity polarity;
bool enabled;
};
@@ -107,13 +107,13 @@ static inline bool pwm_is_enabled(const struct pwm_device *pwm)
return state.enabled;
}
-static inline void pwm_set_period(struct pwm_device *pwm, unsigned int period)
+static inline void pwm_set_period(struct pwm_device *pwm, u64 period)
{
if (pwm)
pwm->state.period = period;
}
-static inline unsigned int pwm_get_period(const struct pwm_device *pwm)
+static inline u64 pwm_get_period(const struct pwm_device *pwm)
{
struct pwm_state state;
@@ -128,7 +128,7 @@ static inline void pwm_set_duty_cycle(struct pwm_device *pwm, unsigned int duty)
pwm->state.duty_cycle = duty;
}
-static inline unsigned int pwm_get_duty_cycle(const struct pwm_device *pwm)
+static inline u64 pwm_get_duty_cycle(const struct pwm_device *pwm)
{
struct pwm_state state;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH v16 06/11] pwm: imx27: Use 64-bit division macro
From: Guru Das Srinagesh @ 2020-06-02 22:34 UTC (permalink / raw)
To: linux-pwm, Thierry Reding, Uwe Kleine-König
Cc: linux-arm-kernel, Daniel Thompson, Arnd Bergmann, David Collins,
Stephen Boyd, linux-kernel, Geert Uytterhoeven, Dan Carpenter,
Joe Perches, Subbaraman Narayanamurthy, Lee Jones, Guenter Roeck
In-Reply-To: <fcc92b6499e8200ad6b02ab13b38e4d310afe0ba.1591135774.git.gurus@codeaurora.org>
On Tue, Jun 02, 2020 at 03:31:11PM -0700, Guru Das Srinagesh wrote:
> Since the PWM framework is switching struct pwm_state.period's
> datatype to u64, prepare for this transition by using
> DIV_ROUND_UP_ULL to handle a 64-bit dividend.
>
> Signed-off-by: Guru Das Srinagesh <gurus@codeaurora.org>
> ---
> drivers/pwm/pwm-imx27.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/pwm/pwm-imx27.c b/drivers/pwm/pwm-imx27.c
> index 732a6f3..c50d453 100644
> --- a/drivers/pwm/pwm-imx27.c
> +++ b/drivers/pwm/pwm-imx27.c
> @@ -202,7 +202,7 @@ static void pwm_imx27_wait_fifo_slot(struct pwm_chip *chip,
> sr = readl(imx->mmio_base + MX3_PWMSR);
> fifoav = FIELD_GET(MX3_PWMSR_FIFOAV, sr);
> if (fifoav == MX3_PWMSR_FIFOAV_4WORDS) {
> - period_ms = DIV_ROUND_UP(pwm_get_period(pwm),
> + period_ms = DIV_ROUND_UP_ULL(pwm_get_period(pwm),
> NSEC_PER_MSEC);
> msleep(period_ms);
>
> --
Please disregard this patch, it was sent by mistake. The correct patch
is [1].
[1]: https://lore.kernel.org/lkml/bb43ff8ad9e0b9884722d85e51c8fdb3ef4154d1.1591136989.git.gurus@codeaurora.org/
Thank you.
Guru Das.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] pwm: imx27: Fix rounding behavior
From: Guru Das Srinagesh @ 2020-06-02 22:36 UTC (permalink / raw)
To: Thierry Reding
Cc: linux-pwm, Shawn Guo, Sascha Hauer, NXP Linux Team,
Pengutronix Kernel Team, Uwe Kleine-König, Fabio Estevam,
linux-arm-kernel
In-Reply-To: <20200602204211.GA1693@codeaurora.org>
On Tue, Jun 02, 2020 at 01:42:12PM -0700, Guru Das Srinagesh wrote:
> On Tue, Jun 02, 2020 at 02:48:35PM +0200, Thierry Reding wrote:
> > On Thu, Apr 16, 2020 at 10:02:45AM +0200, Uwe Kleine-König wrote:
> > > To not trigger the warnings provided by CONFIG_PWM_DEBUG
> > >
> > > - use up-rounding in .get_state()
> > > - don't divide by the result of a division
> > > - don't use the rounded counter value for the period length to calculate
> > > the counter value for the duty cycle
> > >
> > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > > ---
> > > drivers/pwm/pwm-imx27.c | 20 ++++++++++----------
> > > 1 file changed, 10 insertions(+), 10 deletions(-)
> >
> > Applied, thanks.
> >
> > Thierry
>
> Hi Thierry,
>
> Just FYI, This change conflicts with one of my patches [1] in the "Convert
> PWM period and duty cycle to u64" series.
>
> [1]: https://patchwork.ozlabs.org/project/linux-pwm/patch/848494725fd1240ed877d0a1471dd11ccea01ff5.1590514331.git.gurus@codeaurora.org/
Uploaded v16 that resolves this issue.
Thank you.
Guru Das.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 06/10] dt-bindings: spi: spi-dw-mchp: Add Sparx5 support
From: Serge Semin @ 2020-06-02 23:07 UTC (permalink / raw)
To: Lars Povlsen
Cc: devicetree, Alexandre Belloni, Mark Brown, linux-kernel,
Serge Semin, linux-spi, SoC Team, Rob Herring,
Microchip Linux Driver Support, linux-arm-kernel
In-Reply-To: <20200513140031.25633-7-lars.povlsen@microchip.com>
On Wed, May 13, 2020 at 04:00:27PM +0200, Lars Povlsen wrote:
> This add DT bindings for the Sparx5 SPI driver.
This whole file can be easily merged in to the generic DW APB SSI DT
binding file. Just use "if: properties: compatible: const: ..." construction
to distinguish ocelot, jaguar, sparx5 and non-sparx5 nodes.
>
> Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
> ---
> .../bindings/spi/mscc,ocelot-spi.yaml | 49 +++++++++++++++----
> 1 file changed, 39 insertions(+), 10 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/spi/mscc,ocelot-spi.yaml b/Documentation/devicetree/bindings/spi/mscc,ocelot-spi.yaml
> index a3ac0fa576553..8beecde4b0880 100644
> --- a/Documentation/devicetree/bindings/spi/mscc,ocelot-spi.yaml
> +++ b/Documentation/devicetree/bindings/spi/mscc,ocelot-spi.yaml
> @@ -23,15 +23,23 @@ properties:
> enum:
> - mscc,ocelot-spi
> - mscc,jaguar2-spi
> + - microchip,sparx5-spi
>
> interrupts:
> maxItems: 1
>
> reg:
> minItems: 2
> - items:
> - - description: Designware SPI registers
> - - description: CS override registers
> + maxItems: 3
> + oneOf:
> + - items:
> + - description: Designware SPI registers
> + - description: CS override registers (Not sparx5).
> + - items:
> + - description: Designware SPI registers
> + - description: CS override registers (Not sparx5).
> + - description: Direct mapped SPI read area. If provided, the
> + driver will register spi_mem_op's to take advantage of it.
>
> clocks:
> maxItems: 1
> @@ -43,6 +51,23 @@ properties:
> enum: [ 2, 4 ]
> maxItems: 1
>
> + spi-rx-delay-us:
> + description: |
> + The delay (in usec) of the RX signal sample position. This can
> + be used to tne the RX timing in order to acheive higher
> + speeds. This is used for all devices on the bus.
> + default: 0
> + maxItems: 1
spi-rx-delay-us is defined for a particular SPI-slave. Please see the
DT binding file: Documentation/devicetree/bindings/spi/spi-controller.yaml .
Although as I suggested before this delay isn't what the Dw APB SSI RX sample
delay functionality does. Probably a vendor-specific property would be better
here. But I'd also define it on a SPI-slave basis, not for all devices on the
bus.
> +
> + interface-mapping-mask:
> + description: |
> + On the Sparx5 variant, two different busses are connected to the
> + controller. This property is a mask per chip-select, indicating
> + whether the CS should go to one or the other interface.
> + $ref: /schemas/types.yaml#/definitions/uint32
> + default: 0
> + maxItems: 1
As Mark rightfully suggested this seems like an SPI-slave related property, then
most likely it should be defined on the SPI-slave basis (probably as a bool
property). Additionally it's vendor-specific, so the property name should be
accordingly prefixed.
> +
> required:
> - compatible
> - reg
> @@ -50,11 +75,15 @@ required:
>
> examples:
> - |
> - spi0: spi@101000 {
> - compatible = "mscc,ocelot-spi";
> - #address-cells = <1>;
> - #size-cells = <0>;
> - reg = <0x101000 0x100>, <0x3c 0x18>;
> - interrupts = <9>;
> - clocks = <&ahb_clk>;
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> + spi0: spi@600104000 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + compatible = "microchip,sparx5-spi";
> + reg = <0x00104000 0x40>, <0 0>, <0x3000000 0x4000000>;
I have a doubt that defining an empty reg region is a good idea, since you can
detect the reg requirements by the node compatible string.
-Sergey
> + num-cs = <16>;
> + reg-io-width = <4>;
> + reg-shift = <2>;
> + clocks = <&ahb_clk>;
> + interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>;
> };
> --
> 2.26.2
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 07/10] spi: spi-dw-mchp: Add Sparx5 support
From: Serge Semin @ 2020-06-02 23:22 UTC (permalink / raw)
To: Lars Povlsen
Cc: devicetree, Alexandre Belloni, linux-kernel, Serge Semin,
linux-spi, SoC Team, Mark Brown, Microchip Linux Driver Support,
linux-arm-kernel
In-Reply-To: <20200513140031.25633-8-lars.povlsen@microchip.com>
On Wed, May 13, 2020 at 04:00:28PM +0200, Lars Povlsen wrote:
> This adds support for the Sparx5 SoC in the spi-dw-mchp SPI controller.
>
> Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
> ---
> drivers/spi/spi-dw-mchp.c | 211 ++++++++++++++++++++++++++++++++++----
> 1 file changed, 189 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/spi/spi-dw-mchp.c b/drivers/spi/spi-dw-mchp.c
> index 0828a7616d9ab..3abdd44a550ea 100644
> --- a/drivers/spi/spi-dw-mchp.c
> +++ b/drivers/spi/spi-dw-mchp.c
> @@ -28,21 +28,22 @@
>
> #define MAX_CS 4
>
> -#define MSCC_CPU_SYSTEM_CTRL_GENERAL_CTRL 0x24
> -#define OCELOT_IF_SI_OWNER_OFFSET 4
> -#define JAGUAR2_IF_SI_OWNER_OFFSET 6
> #define MSCC_IF_SI_OWNER_MASK GENMASK(1, 0)
> #define MSCC_IF_SI_OWNER_SISL 0
> #define MSCC_IF_SI_OWNER_SIBM 1
> #define MSCC_IF_SI_OWNER_SIMC 2
>
> #define MSCC_SPI_MST_SW_MODE 0x14
> -#define MSCC_SPI_MST_SW_MODE_SW_PIN_CTRL_MODE BIT(13)
> -#define MSCC_SPI_MST_SW_MODE_SW_SPI_CS(x) (x << 5)
>
> struct dw_spi_mchp_props {
> const char *syscon_name;
> - u32 si_owner_bit;
> + u32 general_ctrl_off;
> + u32 si_owner_bit, si_owner2_bit;
> + u32 pinctrl_bit_off;
> + u32 cs_bit_off;
> + u32 ss_force_ena_off;
> + u32 ss_force_val_off;
> + u32 bootmaster_cs;
> };
>
> struct dw_spi_mchp {
> @@ -53,44 +54,176 @@ struct dw_spi_mchp {
> void __iomem *spi_mst;
> const struct dw_spi_mchp_props *props;
> u32 gen_owner;
> + u32 if2mask;
> };
>
> static const struct dw_spi_mchp_props dw_spi_mchp_props_ocelot = {
> .syscon_name = "mscc,ocelot-cpu-syscon",
> + .general_ctrl_off = 0x24,
> .si_owner_bit = 4,
> + .pinctrl_bit_off = 13,
> + .cs_bit_off = 5,
> + .bootmaster_cs = 0,
> };
>
> static const struct dw_spi_mchp_props dw_spi_mchp_props_jaguar2 = {
> .syscon_name = "mscc,ocelot-cpu-syscon",
> + .general_ctrl_off = 0x24,
> .si_owner_bit = 6,
> + .pinctrl_bit_off = 13,
> + .cs_bit_off = 5,
> + .bootmaster_cs = 0,
> +};
> +
> +static const struct dw_spi_mchp_props dw_spi_mchp_props_sparx5 = {
> + .syscon_name = "microchip,sparx5-cpu-syscon",
> + .general_ctrl_off = 0x88,
> + .si_owner_bit = 6,
> + .si_owner2_bit = 4,
> + .ss_force_ena_off = 0xa4,
> + .ss_force_val_off = 0xa8,
> + .bootmaster_cs = 0,
> };
>
> /*
> - * The Designware SPI controller (referred to as master in the documentation)
> - * automatically deasserts chip select when the tx fifo is empty. The chip
> - * selects then needs to be either driven as GPIOs or, for the first 4 using the
> - * the SPI boot controller registers. the final chip select is an OR gate
> - * between the Designware SPI controller and the SPI boot controller.
> + * Set the owner of the SPI interface
> */
> -static void dw_spi_mchp_set_cs(struct spi_device *spi, bool enable)
> +static void dw_spi_mchp_set_owner(struct dw_spi_mchp *dwsmchp,
> + const struct dw_spi_mchp_props *props,
> + u8 owner, u8 owner2)
> +{
> + u32 val, msk;
> +
> + val = (owner << props->si_owner_bit);
> + msk = (MSCC_IF_SI_OWNER_MASK << props->si_owner_bit);
> + if (props->si_owner2_bit) {
> + val |= owner2 << props->si_owner2_bit;
> + msk |= (MSCC_IF_SI_OWNER_MASK << props->si_owner2_bit);
> + }
> + if (dwsmchp->gen_owner != val) {
> + regmap_update_bits(dwsmchp->syscon, props->general_ctrl_off,
> + msk, val);
> + dwsmchp->gen_owner = val;
> + }
> +}
> +
> +static void dw_spi_mchp_set_cs_owner(struct dw_spi_mchp *dwsmchp,
> + const struct dw_spi_mchp_props *props,
> + u8 cs, u8 owner)
> {
> + u8 dummy = (owner == MSCC_IF_SI_OWNER_SIBM ?
> + MSCC_IF_SI_OWNER_SIMC : MSCC_IF_SI_OWNER_SIBM);
> + if (props->si_owner2_bit && (dwsmchp->if2mask & BIT(cs))) {
> + /* SPI2 */
> + dw_spi_mchp_set_owner(dwsmchp, props, dummy, owner);
> + } else {
> + /* SPI1 */
> + dw_spi_mchp_set_owner(dwsmchp, props, owner, dummy);
> + }
> +}
> +
> +/*
> + * The Designware SPI controller (referred to as master in the
> + * documentation) automatically deasserts chip select when the tx fifo
> + * is empty. The chip selects then needs to be either driven as GPIOs
> + * or, for the first 4 using the the SPI boot controller
> + * registers. the final chip select is an OR gate between the
> + * Designware SPI controller and the SPI boot controller. nselect is
> + * an active low signal
> + */
> +static void dw_spi_mchp_set_cs(struct spi_device *spi, bool nEnable)
> +{
> + bool enable = !nEnable; /* This keeps changing in the API... */
> struct dw_spi *dws = spi_master_get_devdata(spi->master);
> struct dw_spi_mchp *dwsmchp = container_of(dws, struct dw_spi_mchp,
> dws);
> - u32 cs = spi->chip_select;
> + const struct dw_spi_mchp_props *props = dwsmchp->props;
> + u8 cs = spi->chip_select;
>
> - if (cs < 4) {
> - u32 sw_mode = MSCC_SPI_MST_SW_MODE_SW_PIN_CTRL_MODE;
> + if (enable)
> + dw_spi_mchp_set_cs_owner(dwsmchp, props, cs,
> + MSCC_IF_SI_OWNER_SIMC);
>
> - if (!enable)
> - sw_mode |= MSCC_SPI_MST_SW_MODE_SW_SPI_CS(BIT(cs));
> + if (dwsmchp->spi_mst && (cs < MAX_CS)) {
> + u32 sw_mode;
>
> + if (enable)
> + sw_mode = BIT(props->pinctrl_bit_off) |
> + (BIT(cs) << props->cs_bit_off);
> + else
> + sw_mode = 0;
> writel(sw_mode, dwsmchp->spi_mst + MSCC_SPI_MST_SW_MODE);
> + } else if (props->ss_force_ena_off) {
> + if (enable) {
> + /* Ensure CS toggles, so start off all disabled */
> + regmap_write(dwsmchp->syscon, props->ss_force_val_off,
> + ~0);
> + /* CS override drive enable */
> + regmap_write(dwsmchp->syscon, props->ss_force_ena_off,
> + 1);
> + /* Allow settle */
> + udelay(1);
> + /* Now set CSx enabled */
> + regmap_write(dwsmchp->syscon, props->ss_force_val_off,
> + ~BIT(cs));
> + } else {
> + /* CS value */
> + regmap_write(dwsmchp->syscon, props->ss_force_val_off,
> + ~0);
> + /* CS override drive disable */
> + regmap_write(dwsmchp->syscon, props->ss_force_ena_off,
> + 0);
> + }
> }
>
> - dw_spi_set_cs(spi, enable);
> + dw_spi_set_cs(spi, nEnable);
> +}
> +
> +static int dw_mchp_bootmaster_exec_mem_op(struct spi_mem *mem,
> + const struct spi_mem_op *op)
> +{
> + struct spi_device *spi = mem->spi;
> + int ret = -ENOTSUPP;
> +
> + /* Only reads, addrsize 1..4 */
> + if (!op->data.nbytes || !op->addr.nbytes || op->addr.nbytes > 4 ||
> + op->data.dir != SPI_MEM_DATA_IN)
> + return ret;
> +
> + /* Only handle (normal+fast) 3/4 bytes read */
> + if (op->cmd.opcode != SPINOR_OP_READ &&
> + op->cmd.opcode != SPINOR_OP_READ_FAST &&
> + op->cmd.opcode != SPINOR_OP_READ_4B &&
> + op->cmd.opcode != SPINOR_OP_READ_FAST_4B)
> + return ret;
Hm, this part most like belongs to supports_op() callback.
> +
> + /* CS0..3, only 16M reach */
> + if ((spi->chip_select < MAX_CS) &&
> + (op->addr.val + op->data.nbytes) < SZ_16M) {
The driver shouldn't return a failure if more than available data requested.
Just return the length the driver managed to read.
> + struct dw_spi *dws = spi_master_get_devdata(spi->master);
> + struct dw_spi_mchp *dwsmchp = container_of(dws,
> + struct dw_spi_mchp,
> + dws);
> + const struct dw_spi_mchp_props *props = dwsmchp->props;
> + u8 __iomem *src = dwsmchp->read_map +
> + (spi->chip_select * SZ_16M) + op->addr.val;
> +
> + if (props->bootmaster_cs != spi->chip_select)
> + return ret;
> +
> + /* Make boot master owner of SI interface */
> + dw_spi_mchp_set_cs_owner(dwsmchp, props, spi->chip_select,
> + MSCC_IF_SI_OWNER_SIBM);
> + memcpy(op->data.buf.in, src, op->data.nbytes);
So after all it's just memcpy from the directly mapped SPI-flash memory, right?
Then it's not mem_op, but I supposed it should be implemented by means of the
dirmap_{create,read,destroy}.
-Sergey
> + ret = op->data.nbytes;
> + }
> + return ret;
> }
>
> +static const struct spi_controller_mem_ops dw_mchp_bootmaster_mem_ops = {
> + .exec_op = dw_mchp_bootmaster_exec_mem_op,
> +};
> +
> static int dw_spi_mchp_init(struct platform_device *pdev,
> struct dw_spi *dws,
> struct dw_spi_mchp *dwsmchp,
> @@ -107,6 +240,18 @@ static int dw_spi_mchp_init(struct platform_device *pdev,
> }
> }
>
> + /* See if we have a direct read window */
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 2);
> + if (res && resource_size(res) >= (SZ_16M*MAX_CS)) {
> + void __iomem *ptr = devm_ioremap_resource(&pdev->dev, res);
> +
> + if (!IS_ERR(ptr)) {
> + dwsmchp->read_map = ptr;
> + dws->mem_ops = &dw_mchp_bootmaster_mem_ops;
> + dev_info(&pdev->dev, "Enabling fast memory operations\n");
> + }
> + }
> +
> dwsmchp->syscon =
> syscon_regmap_lookup_by_compatible(props->syscon_name);
> if (IS_ERR(dwsmchp->syscon)) {
> @@ -119,10 +264,9 @@ static int dw_spi_mchp_init(struct platform_device *pdev,
> if (dwsmchp->spi_mst)
> writel(0, dwsmchp->spi_mst + MSCC_SPI_MST_SW_MODE);
>
> - /* Select the owner of the SI interface */
> - regmap_update_bits(dwsmchp->syscon, MSCC_CPU_SYSTEM_CTRL_GENERAL_CTRL,
> - MSCC_IF_SI_OWNER_MASK << props->si_owner_bit,
> - MSCC_IF_SI_OWNER_SIMC << props->si_owner_bit);
> + /* SPI2 mapping bitmask */
> + device_property_read_u32(&pdev->dev, "interface-mapping-mask",
> + &dwsmchp->if2mask);
>
> dwsmchp->dws.set_cs = dw_spi_mchp_set_cs;
>
> @@ -180,6 +324,27 @@ static int dw_spi_mchp_probe(struct platform_device *pdev)
> dws->rx_sample_dly = DIV_ROUND_UP(rx_sample_dly,
> (dws->max_freq / 1000000));
>
> + if (pdev->dev.of_node) {
> + int i;
> +
> + for (i = 0; i < dws->num_cs; i++) {
> + int cs_gpio = of_get_named_gpio(pdev->dev.of_node,
> + "cs-gpios", i);
> +
> + if (cs_gpio == -EPROBE_DEFER) {
> + ret = cs_gpio;
> + goto out;
> + }
> +
> + if (gpio_is_valid(cs_gpio)) {
> + ret = devm_gpio_request(&pdev->dev, cs_gpio,
> + dev_name(&pdev->dev));
> + if (ret)
> + goto out;
> + }
> + }
> + }
> +
> props = device_get_match_data(&pdev->dev);
> if (props)
> ret = dw_spi_mchp_init(pdev, dws, dwsmchp, props);
> @@ -213,6 +378,8 @@ static int dw_spi_mchp_remove(struct platform_device *pdev)
> static const struct of_device_id dw_spi_mchp_of_match[] = {
> { .compatible = "mscc,ocelot-spi", .data = &dw_spi_mchp_props_ocelot},
> { .compatible = "mscc,jaguar2-spi", .data = &dw_spi_mchp_props_jaguar2},
> + { .compatible = "microchip,sparx5-spi",
> + .data = &dw_spi_mchp_props_sparx5},
> { /* end of table */}
> };
> MODULE_DEVICE_TABLE(of, dw_spi_mchp_of_match);
> --
> 2.26.2
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [soc:baikal/drivers-1 4/5] drivers/bus/bt1-apb.c:330:3: error: implicit declaration of function 'readl'
From: kbuild test robot @ 2020-06-02 23:21 UTC (permalink / raw)
To: Serge, Semin,; +Cc: arm, kbuild-all, Arnd Bergmann, linux-arm-kernel
[-- Attachment #1: Type: text/plain, Size: 1995 bytes --]
tree: https://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git baikal/drivers-1
head: 83ca8b3e8f213f49cc68b5c1fbcf88ebb24671eb
commit: 8f93662d8324940e8925a0e492c587dbcf7c7fee [4/5] bus: Add Baikal-T1 APB-bus driver
config: sparc64-allyesconfig (attached as .config)
compiler: sparc64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
git checkout 8f93662d8324940e8925a0e492c587dbcf7c7fee
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=sparc64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>
All errors (new ones prefixed by >>, old ones prefixed by <<):
drivers/bus/bt1-apb.c: In function 'inject_error_store':
>> drivers/bus/bt1-apb.c:330:3: error: implicit declaration of function 'readl' [-Werror=implicit-function-declaration]
330 | readl(apb->res);
| ^~~~~
cc1: some warnings being treated as errors
vim +/readl +330 drivers/bus/bt1-apb.c
318
319 static ssize_t inject_error_store(struct device *dev,
320 struct device_attribute *attr,
321 const char *data, size_t count)
322 {
323 struct bt1_apb *apb = dev_get_drvdata(dev);
324
325 /*
326 * Either dummy read from the unmapped address in the APB IO area
327 * or manually set the IRQ status.
328 */
329 if (!strncmp(data, "nodev", 5))
> 330 readl(apb->res);
331 else if (!strncmp(data, "irq", 3))
332 regmap_update_bits(apb->regs, APB_EHB_ISR, APB_EHB_ISR_PENDING,
333 APB_EHB_ISR_PENDING);
334 else
335 return -EINVAL;
336
337 return count;
338 }
339 static DEVICE_ATTR_RW(inject_error);
340
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 62590 bytes --]
[-- Attachment #3: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 1/7] drm/rockchip: prepare common code for cdns and rk dpi/dp driver
From: Laurent Pinchart @ 2020-06-02 23:28 UTC (permalink / raw)
To: Emil Velikov
Cc: Jernej Skrabec, Heiko Stübner, Jonas Karlman, sandor.yu,
Neil Armstrong, Sandy Huang, ML dri-devel,
Linux-Kernel@Vger. Kernel. Org, Andrzej Hajda, NXP Linux Team,
linux-rockchip, dkos, LAKML
In-Reply-To: <CACvgo52NeUSQV5p8+4DkCjpkv12cs8fCkQqy4MFn8pVaorVaHg@mail.gmail.com>
On Tue, Jun 02, 2020 at 02:55:52PM +0100, Emil Velikov wrote:
> On Mon, 1 Jun 2020 at 07:29, <sandor.yu@nxp.com> wrote:
> >
> > From: Sandor Yu <Sandor.yu@nxp.com>
> >
> > - Extracted common fields from cdn_dp_device to a new cdns_mhdp_device
> > structure which will be used by two separate drivers later on.
> > - Moved some datatypes (audio_format, audio_info, vic_pxl_encoding_format,
> > video_info) from cdn-dp-core.c to cdn-dp-reg.h.
> > - Changed prefixes from cdn_dp to cdns_mhdp
> > cdn -> cdns to match the other Cadence's drivers
> > dp -> mhdp to distinguish it from a "just a DP" as the IP underneath
> > this registers map can be a HDMI (which is internally different,
> > but the interface for commands, events is pretty much the same).
> > - Modified cdn-dp-core.c to use the new driver structure and new function
> > names.
> > - writel and readl are replaced by cdns_mhdp_bus_write and
> > cdns_mhdp_bus_read.
> >
> The high-level idea is great - split, refactor and reuse the existing drivers.
>
> Although looking at the patches themselves - they seems to be doing
> multiple things at once.
> As indicated by the extensive list in the commit log.
>
> I would suggest splitting those up a bit, roughly in line of the
> itemisation as per the commit message.
>
> Here is one hand wavy way to chunk this patch:
> 1) use put_unalligned*
> 2) 'use local variable dev' style of changes (as seem in cdn_dp_clk_enable)
> 3) add writel/readl wrappers
> 4) hookup struct cdns_mhdp_device, keep dp->mhdp detail internal.
> The cdn-dp-reg.h function names/signatures will stay the same.
> 5) finalize the helpers - use mhdp directly, rename
I second this, otherwise review is very hard.
> Examples:
> 4)
> static int cdn_dp_mailbox_read(struct cdn_dp_device *dp)
> {
> +" struct cdns_mhdp_device *mhdp = dp->mhdp;
> int val, ret;
>
> - ret = readx_poll_timeout(readl, dp->regs + MAILBOX_EMPTY_ADDR,
> + ret = readx_poll_timeout(readl, mhdp->regs_base + MAILBOX_EMPTY_ADDR,
> ...
> return fancy_readl(dp, MAILBOX0_RD_DATA) & 0xff;
> }
>
> 5)
> -static int cdn_dp_mailbox_read(struct cdn_dp_device *dp)
> +static int mhdp_mailbox_read(struct cdns_mhdp_device *mhdp)
> {
> - struct cdns_mhdp_device *mhdp = dp->mhdp;
> int val, ret;
> ...
> - return fancy_readl(dp, MAILBOX0_RD_DATA) & 0xff;
> + return cdns_mhdp_bus_read(mhdp, MAILBOX0_RD_DATA) & 0xff;
> }
--
Regards,
Laurent Pinchart
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 3/7] drm: bridge: cadence: initial support for MHDP DP bridge driver
From: Laurent Pinchart @ 2020-06-02 23:35 UTC (permalink / raw)
To: sandor.yu
Cc: jernej.skrabec, heiko, jonas, narmstrong, hjc, dri-devel,
linux-kernel, a.hajda, linux-imx, linux-rockchip, dkos,
linux-arm-kernel
In-Reply-To: <cebdfdaffa40e208f925661522fa9ee1fb0f8721.1590982881.git.Sandor.yu@nxp.com>
Hi Sandor,
Thank you for the patch.
On Mon, Jun 01, 2020 at 02:17:33PM +0800, sandor.yu@nxp.com wrote:
> From: Sandor Yu <Sandor.yu@nxp.com>
>
> This adds initial support for MHDP DP bridge driver.
> Basic DP functions are supported, that include:
> -Video mode set on-the-fly
> -Cable hotplug detect
> -MAX support resolution to 3096x2160@60fps
> -Support DP audio
> -EDID read via AUX
>
> Signed-off-by: Sandor Yu <Sandor.yu@nxp.com>
> ---
> drivers/gpu/drm/bridge/cadence/Kconfig | 4 +
> drivers/gpu/drm/bridge/cadence/Makefile | 1 +
> drivers/gpu/drm/bridge/cadence/cdns-dp-core.c | 530 ++++++++++++++++++
> .../gpu/drm/bridge/cadence/cdns-mhdp-audio.c | 100 ++++
> .../gpu/drm/bridge/cadence/cdns-mhdp-common.c | 42 +-
> .../gpu/drm/bridge/cadence/cdns-mhdp-common.h | 3 +
> drivers/gpu/drm/bridge/cadence/cdns-mhdp-dp.c | 34 +-
> drivers/gpu/drm/rockchip/cdn-dp-core.c | 7 +-
> include/drm/bridge/cdns-mhdp.h | 52 +-
> 9 files changed, 740 insertions(+), 33 deletions(-)
> create mode 100644 drivers/gpu/drm/bridge/cadence/cdns-dp-core.c
>
> diff --git a/drivers/gpu/drm/bridge/cadence/Kconfig b/drivers/gpu/drm/bridge/cadence/Kconfig
> index 48c1b0f77dc6..b7b8d30b18b6 100644
> --- a/drivers/gpu/drm/bridge/cadence/Kconfig
> +++ b/drivers/gpu/drm/bridge/cadence/Kconfig
> @@ -5,3 +5,7 @@ config DRM_CDNS_MHDP
> depends on OF
> help
> Support Cadence MHDP API library.
> +
> +config DRM_CDNS_DP
> + tristate "Cadence DP DRM driver"
> + depends on DRM_CDNS_MHDP
> diff --git a/drivers/gpu/drm/bridge/cadence/Makefile b/drivers/gpu/drm/bridge/cadence/Makefile
> index ddb2ba4fb852..cb3c88311a64 100644
> --- a/drivers/gpu/drm/bridge/cadence/Makefile
> +++ b/drivers/gpu/drm/bridge/cadence/Makefile
> @@ -1,3 +1,4 @@
> # SPDX-License-Identifier: GPL-2.0-only
> cdns_mhdp_drmcore-y := cdns-mhdp-common.o cdns-mhdp-audio.o cdns-mhdp-dp.o
> +cdns_mhdp_drmcore-$(CONFIG_DRM_CDNS_DP) += cdns-dp-core.o
> obj-$(CONFIG_DRM_CDNS_MHDP) += cdns_mhdp_drmcore.o
> diff --git a/drivers/gpu/drm/bridge/cadence/cdns-dp-core.c b/drivers/gpu/drm/bridge/cadence/cdns-dp-core.c
> new file mode 100644
> index 000000000000..b2fe8fdc64ed
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/cadence/cdns-dp-core.c
> @@ -0,0 +1,530 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Cadence Display Port Interface (DP) driver
> + *
> + * Copyright (C) 2019-2020 NXP Semiconductor, Inc.
> + *
> + */
> +#include <drm/bridge/cdns-mhdp.h>
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_crtc_helper.h>
> +#include <drm/drm_edid.h>
> +#include <drm/drm_encoder_slave.h>
> +#include <drm/drm_of.h>
> +#include <drm/drm_probe_helper.h>
> +#include <drm/drm_vblank.h>
> +#include <drm/drm_print.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/irq.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/of_device.h>
> +
> +#include "cdns-mhdp-common.h"
> +
> +/*
> + * This function only implements native DPDC reads and writes
> + */
> +static ssize_t dp_aux_transfer(struct drm_dp_aux *aux,
> + struct drm_dp_aux_msg *msg)
> +{
> + struct cdns_mhdp_device *mhdp = dev_get_drvdata(aux->dev);
> + bool native = msg->request & (DP_AUX_NATIVE_WRITE & DP_AUX_NATIVE_READ);
> + int ret;
> +
> + /* Ignore address only message */
> + if ((msg->size == 0) || (msg->buffer == NULL)) {
> + msg->reply = native ?
> + DP_AUX_NATIVE_REPLY_ACK : DP_AUX_I2C_REPLY_ACK;
> + return msg->size;
> + }
> +
> + if (!native) {
> + dev_err(mhdp->dev, "%s: only native messages supported\n", __func__);
> + return -EINVAL;
> + }
> +
> + /* msg sanity check */
> + if (msg->size > DP_AUX_MAX_PAYLOAD_BYTES) {
> + dev_err(mhdp->dev, "%s: invalid msg: size(%zu), request(%x)\n",
> + __func__, msg->size, (unsigned int)msg->request);
> + return -EINVAL;
> + }
> +
> + if (msg->request == DP_AUX_NATIVE_WRITE) {
> + const u8 *buf = msg->buffer;
> + int i;
> + for (i = 0; i < msg->size; ++i) {
> + ret = cdns_mhdp_dpcd_write(mhdp,
> + msg->address + i, buf[i]);
> + if (!ret)
> + continue;
> +
> + DRM_DEV_ERROR(mhdp->dev, "Failed to write DPCD\n");
> +
> + return ret;
> + }
> + msg->reply = DP_AUX_NATIVE_REPLY_ACK;
> + return msg->size;
> + }
> +
> + if (msg->request == DP_AUX_NATIVE_READ) {
> + ret = cdns_mhdp_dpcd_read(mhdp, msg->address, msg->buffer, msg->size);
> + if (ret < 0)
> + return -EIO;
> + msg->reply = DP_AUX_NATIVE_REPLY_ACK;
> + return msg->size;
> + }
> + return 0;
> +}
> +
> +static int dp_aux_init(struct cdns_mhdp_device *mhdp,
> + struct device *dev)
> +{
> + int ret;
> +
> + mhdp->dp.aux.name = "imx_dp_aux";
> + mhdp->dp.aux.dev = dev;
> + mhdp->dp.aux.transfer = dp_aux_transfer;
> +
> + ret = drm_dp_aux_register(&mhdp->dp.aux);
> +
> + return ret;
> +}
> +
> +static int dp_aux_destroy(struct cdns_mhdp_device *mhdp)
> +{
> + drm_dp_aux_unregister(&mhdp->dp.aux);
> + return 0;
> +}
> +
> +static void dp_pixel_clk_reset(struct cdns_mhdp_device *mhdp)
> +{
> + u32 val;
> +
> + /* reset pixel clk */
> + val = cdns_mhdp_reg_read(mhdp, SOURCE_HDTX_CAR);
> + cdns_mhdp_reg_write(mhdp, SOURCE_HDTX_CAR, val & 0xFD);
> + cdns_mhdp_reg_write(mhdp, SOURCE_HDTX_CAR, val);
> +}
> +
> +static void cdns_dp_mode_set(struct cdns_mhdp_device *mhdp)
> +{
> + int ret;
> +
> + cdns_mhdp_plat_call(mhdp, pclk_rate);
> +
> + /* delay for DP FW stable after pixel clock relock */
> + msleep(50);
> +
> + dp_pixel_clk_reset(mhdp);
> +
> + /* Get DP Caps */
> + ret = drm_dp_dpcd_read(&mhdp->dp.aux, DP_DPCD_REV, mhdp->dp.dpcd,
> + DP_RECEIVER_CAP_SIZE);
> + if (ret < 0) {
> + DRM_ERROR("Failed to get caps %d\n", ret);
> + return;
> + }
> +
> + mhdp->dp.rate = drm_dp_max_link_rate(mhdp->dp.dpcd);
> + mhdp->dp.num_lanes = drm_dp_max_lane_count(mhdp->dp.dpcd);
> +
> + /* check the max link rate */
> + if (mhdp->dp.rate > CDNS_DP_MAX_LINK_RATE)
> + mhdp->dp.rate = CDNS_DP_MAX_LINK_RATE;
> +
> + /* Initialize link rate/num_lanes as panel max link rate/max_num_lanes */
> + cdns_mhdp_plat_call(mhdp, phy_set);
> +
> + /* Video off */
> + ret = cdns_mhdp_set_video_status(mhdp, CONTROL_VIDEO_IDLE);
> + if (ret) {
> + DRM_DEV_ERROR(mhdp->dev, "Failed to valid video %d\n", ret);
> + return;
> + }
> +
> + /* Line swaping */
> + mhdp->lane_mapping = mhdp->plat_data->lane_mapping;
> + cdns_mhdp_reg_write(mhdp, LANES_CONFIG, 0x00400000 | mhdp->lane_mapping);
> +
> + /* Set DP host capability */
> + ret = cdns_mhdp_set_host_cap(mhdp);
> + if (ret) {
> + DRM_DEV_ERROR(mhdp->dev, "Failed to set host cap %d\n", ret);
> + return;
> + }
> +
> + ret = cdns_mhdp_config_video(mhdp);
> + if (ret) {
> + DRM_DEV_ERROR(mhdp->dev, "Failed to config video %d\n", ret);
> + return;
> + }
> +
> + return;
> +}
> +
> +/* -----------------------------------------------------------------------------
> + * DP TX Setup
> + */
> +static enum drm_connector_status
> +cdns_dp_connector_detect(struct drm_connector *connector, bool force)
> +{
> + struct cdns_mhdp_device *mhdp = container_of(connector,
> + struct cdns_mhdp_device, connector.base);
> + u8 hpd = 0xf;
> +
> + hpd = cdns_mhdp_read_hpd(mhdp);
> + if (hpd == 1)
> + /* Cable Connected */
> + return connector_status_connected;
> + else if (hpd == 0)
> + /* Cable Disconnedted */
> + return connector_status_disconnected;
> + else {
> + /* Cable status unknown */
> + DRM_INFO("Unknow cable status, hdp=%u\n", hpd);
> + return connector_status_unknown;
> + }
> +}
> +
> +static int cdns_dp_connector_get_modes(struct drm_connector *connector)
> +{
> + struct cdns_mhdp_device *mhdp = container_of(connector,
> + struct cdns_mhdp_device, connector.base);
> + int num_modes = 0;
> + struct edid *edid;
> +
> + edid = drm_do_get_edid(&mhdp->connector.base,
> + cdns_mhdp_get_edid_block, mhdp);
> + if (edid) {
> + dev_info(mhdp->dev, "%x,%x,%x,%x,%x,%x,%x,%x\n",
> + edid->header[0], edid->header[1],
> + edid->header[2], edid->header[3],
> + edid->header[4], edid->header[5],
> + edid->header[6], edid->header[7]);
> + drm_connector_update_edid_property(connector, edid);
> + num_modes = drm_add_edid_modes(connector, edid);
> + kfree(edid);
> + }
> +
> + if (num_modes == 0)
> + DRM_ERROR("Invalid edid\n");
> + return num_modes;
> +}
> +
> +static const struct drm_connector_funcs cdns_dp_connector_funcs = {
> + .fill_modes = drm_helper_probe_single_connector_modes,
> + .detect = cdns_dp_connector_detect,
> + .destroy = drm_connector_cleanup,
> + .reset = drm_atomic_helper_connector_reset,
> + .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> + .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> +};
> +
> +static const struct drm_connector_helper_funcs cdns_dp_connector_helper_funcs = {
> + .get_modes = cdns_dp_connector_get_modes,
> +};
> +
> +static int cdns_dp_bridge_attach(struct drm_bridge *bridge,
> + enum drm_bridge_attach_flags flags)
> +{
> + struct cdns_mhdp_device *mhdp = bridge->driver_private;
> + struct drm_encoder *encoder = bridge->encoder;
> + struct drm_connector *connector = &mhdp->connector.base;
New bridge drivers need to support operation without creating a
connector when the DRM_BRIDGE_ATTACH_NO_CONNECTOR flag is set in the
flags argument. For an example of how this is done, please see
https://lore.kernel.org/dri-devel/20200526011505.31884-23-laurent.pinchart+renesas@ideasonboard.com/
> +
> + connector->interlace_allowed = 1;
> +
> + connector->polled = DRM_CONNECTOR_POLL_HPD;
> +
> + drm_connector_helper_add(connector, &cdns_dp_connector_helper_funcs);
> +
> + drm_connector_init(bridge->dev, connector, &cdns_dp_connector_funcs,
> + DRM_MODE_CONNECTOR_DisplayPort);
> +
> + drm_connector_attach_encoder(connector, encoder);
> +
> + return 0;
> +}
> +
> +static enum drm_mode_status
> +cdns_dp_bridge_mode_valid(struct drm_bridge *bridge,
> + const struct drm_display_mode *mode)
> +{
> + enum drm_mode_status mode_status = MODE_OK;
> +
> + /* We don't support double-clocked modes */
> + if (mode->flags & DRM_MODE_FLAG_DBLCLK ||
> + mode->flags & DRM_MODE_FLAG_INTERLACE)
> + return MODE_BAD;
> +
> + /* MAX support pixel clock rate 594MHz */
> + if (mode->clock > 594000)
> + return MODE_CLOCK_HIGH;
> +
> + /* 4096x2160 is not supported */
> + if (mode->hdisplay > 3840)
> + return MODE_BAD_HVALUE;
> +
> + if (mode->vdisplay > 2160)
> + return MODE_BAD_VVALUE;
> +
> + return mode_status;
> +}
> +
> +static void cdns_dp_bridge_mode_set(struct drm_bridge *bridge,
> + const struct drm_display_mode *orig_mode,
> + const struct drm_display_mode *mode)
> +{
> + struct cdns_mhdp_device *mhdp = bridge->driver_private;
> + struct drm_display_info *display_info = &mhdp->connector.base.display_info;
> + struct video_info *video = &mhdp->video_info;
> +
> + switch (display_info->bpc) {
> + case 10:
> + video->color_depth = 10;
> + break;
> + case 6:
> + video->color_depth = 6;
> + break;
> + default:
> + video->color_depth = 8;
> + break;
> + }
> +
> + video->color_fmt = PXL_RGB;
> + video->v_sync_polarity = !!(mode->flags & DRM_MODE_FLAG_NVSYNC);
> + video->h_sync_polarity = !!(mode->flags & DRM_MODE_FLAG_NHSYNC);
> +
> + DRM_INFO("Mode: %dx%dp%d\n", mode->hdisplay, mode->vdisplay, mode->clock);
> + memcpy(&mhdp->mode, mode, sizeof(struct drm_display_mode));
> +
> + mutex_lock(&mhdp->lock);
> + cdns_dp_mode_set(mhdp);
> + mutex_unlock(&mhdp->lock);
> +}
> +
> +static void cdn_dp_bridge_enable(struct drm_bridge *bridge)
> +{
> + struct cdns_mhdp_device *mhdp = bridge->driver_private;
> + int ret;
> +
> + /* Link trainning */
> + ret = cdns_mhdp_train_link(mhdp);
> + if (ret) {
> + DRM_DEV_ERROR(mhdp->dev, "Failed link train %d\n", ret);
> + return;
> + }
> +
> + ret = cdns_mhdp_set_video_status(mhdp, CONTROL_VIDEO_VALID);
> + if (ret) {
> + DRM_DEV_ERROR(mhdp->dev, "Failed to valid video %d\n", ret);
> + return;
> + }
> +}
> +
> +static void cdn_dp_bridge_disable(struct drm_bridge *bridge)
> +{
> + struct cdns_mhdp_device *mhdp = bridge->driver_private;
> +
> + cdns_mhdp_set_video_status(mhdp, CONTROL_VIDEO_IDLE);
> +}
> +
> +static const struct drm_bridge_funcs cdns_dp_bridge_funcs = {
> + .attach = cdns_dp_bridge_attach,
> + .enable = cdn_dp_bridge_enable,
> + .disable = cdn_dp_bridge_disable,
> + .mode_set = cdns_dp_bridge_mode_set,
> + .mode_valid = cdns_dp_bridge_mode_valid,
> +};
> +
> +static void hotplug_work_func(struct work_struct *work)
> +{
> + struct cdns_mhdp_device *mhdp = container_of(work,
> + struct cdns_mhdp_device, hotplug_work.work);
> + struct drm_connector *connector = &mhdp->connector.base;
> +
> + drm_helper_hpd_irq_event(connector->dev);
> +
> + if (connector->status == connector_status_connected) {
> + /* Cable connedted */
> + DRM_INFO("HDMI/DP Cable Plug In\n");
> + enable_irq(mhdp->irq[IRQ_OUT]);
> + } else if (connector->status == connector_status_disconnected) {
> + /* Cable Disconnedted */
> + DRM_INFO("HDMI/DP Cable Plug Out\n");
> + enable_irq(mhdp->irq[IRQ_IN]);
> + }
> +}
> +
> +static irqreturn_t cdns_dp_irq_thread(int irq, void *data)
> +{
> + struct cdns_mhdp_device *mhdp = data;
> +
> + disable_irq_nosync(irq);
> +
> + mod_delayed_work(system_wq, &mhdp->hotplug_work,
> + msecs_to_jiffies(HOTPLUG_DEBOUNCE_MS));
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int __cdns_dp_probe(struct platform_device *pdev,
> + struct cdns_mhdp_device *mhdp)
> +{
> + struct device *dev = &pdev->dev;
> + struct resource *iores = NULL;
> + int ret;
> +
> + mutex_init(&mhdp->lock);
> +
> + INIT_DELAYED_WORK(&mhdp->hotplug_work, hotplug_work_func);
> +
> + iores = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (iores) {
> + mhdp->regs_base = devm_ioremap(dev, iores->start,
> + resource_size(iores));
> + if (IS_ERR(mhdp->regs_base))
> + return -ENOMEM;
> + }
> +
> + mhdp->irq[IRQ_IN] = platform_get_irq_byname(pdev, "plug_in");
> + if (mhdp->irq[IRQ_IN] < 0) {
> + dev_info(dev, "No plug_in irq number\n");
> + }
> +
> + mhdp->irq[IRQ_OUT] = platform_get_irq_byname(pdev, "plug_out");
> + if (mhdp->irq[IRQ_OUT] < 0) {
> + dev_info(dev, "No plug_out irq number\n");
> + }
> +
> + cdns_mhdp_plat_call(mhdp, power_on);
> +
> + cdns_mhdp_plat_call(mhdp, firmware_init);
> +
> + /* DP FW alive check */
> + ret = cdns_mhdp_check_alive(mhdp);
> + if (ret == false) {
> + DRM_ERROR("NO dp FW running\n");
> + return -ENXIO;
> + }
> +
> + /* DP PHY init before AUX init */
> + cdns_mhdp_plat_call(mhdp, phy_set);
> +
> + /* Enable Hotplug Detect IRQ thread */
> + irq_set_status_flags(mhdp->irq[IRQ_IN], IRQ_NOAUTOEN);
> + ret = devm_request_threaded_irq(dev, mhdp->irq[IRQ_IN],
> + NULL, cdns_dp_irq_thread,
> + IRQF_ONESHOT, dev_name(dev),
> + mhdp);
> +
> + if (ret) {
> + dev_err(dev, "can't claim irq %d\n",
> + mhdp->irq[IRQ_IN]);
> + return -EINVAL;
> + }
> +
> + irq_set_status_flags(mhdp->irq[IRQ_OUT], IRQ_NOAUTOEN);
> + ret = devm_request_threaded_irq(dev, mhdp->irq[IRQ_OUT],
> + NULL, cdns_dp_irq_thread,
> + IRQF_ONESHOT, dev_name(dev),
> + mhdp);
> +
> + if (ret) {
> + dev_err(dev, "can't claim irq %d\n",
> + mhdp->irq[IRQ_OUT]);
> + return -EINVAL;
> + }
> +
> + if (cdns_mhdp_read_hpd(mhdp))
> + enable_irq(mhdp->irq[IRQ_OUT]);
> + else
> + enable_irq(mhdp->irq[IRQ_IN]);
> +
> + mhdp->bridge.base.driver_private = mhdp;
> + mhdp->bridge.base.funcs = &cdns_dp_bridge_funcs;
> +#ifdef CONFIG_OF
> + mhdp->bridge.base.of_node = dev->of_node;
> +#endif
> +
> + dev_set_drvdata(dev, mhdp);
> +
> + /* register audio driver */
> + cdns_mhdp_register_audio_driver(dev);
> +
> + dp_aux_init(mhdp, dev);
> +
> + return 0;
> +}
> +
> +static void __cdns_dp_remove(struct cdns_mhdp_device *mhdp)
> +{
> + dp_aux_destroy(mhdp);
> + cdns_mhdp_unregister_audio_driver(mhdp->dev);
> +}
> +
> +/* -----------------------------------------------------------------------------
> + * Probe/remove API, used from platforms based on the DRM bridge API.
> + */
> +int cdns_dp_probe(struct platform_device *pdev,
> + struct cdns_mhdp_device *mhdp)
> +{
> + int ret;
> +
> + ret = __cdns_dp_probe(pdev, mhdp);
> + if (ret)
> + return ret;
> +
> + drm_bridge_add(&mhdp->bridge.base);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(cdns_dp_probe);
> +
> +void cdns_dp_remove(struct platform_device *pdev)
> +{
> + struct cdns_mhdp_device *mhdp = platform_get_drvdata(pdev);
> +
> + drm_bridge_remove(&mhdp->bridge.base);
> +
> + __cdns_dp_remove(mhdp);
> +}
> +EXPORT_SYMBOL_GPL(cdns_dp_remove);
> +
> +/* -----------------------------------------------------------------------------
> + * Bind/unbind API, used from platforms based on the component framework.
> + */
> +int cdns_dp_bind(struct platform_device *pdev, struct drm_encoder *encoder,
> + struct cdns_mhdp_device *mhdp)
> +{
> + int ret;
> +
> + ret = __cdns_dp_probe(pdev, mhdp);
> + if (ret < 0)
> + return ret;
> +
> + ret = drm_bridge_attach(encoder, &mhdp->bridge.base, NULL, 0);
> + if (ret) {
> + cdns_dp_remove(pdev);
> + DRM_ERROR("Failed to initialize bridge with drm\n");
> + return ret;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(cdns_dp_bind);
> +
> +void cdns_dp_unbind(struct device *dev)
> +{
> + struct cdns_mhdp_device *mhdp = dev_get_drvdata(dev);
> +
> + __cdns_dp_remove(mhdp);
> +}
> +EXPORT_SYMBOL_GPL(cdns_dp_unbind);
Do we really need to support the component framework, can't we just
support the DRM bridge API ?
> +
> +MODULE_AUTHOR("Sandor Yu <sandor.yu@nxp.com>");
> +MODULE_DESCRIPTION("Cadence Display Port transmitter driver");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:cdns-dp");
> diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp-audio.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp-audio.c
> index 8f51de0672ab..fdd4bcd0d33c 100644
> --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp-audio.c
> +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp-audio.c
> @@ -2,6 +2,9 @@
> /*
> * Copyright (C) Fuzhou Rockchip Electronics Co.Ltd
> * Author: Chris Zhong <zyw@rock-chips.com>
> + *
> + * Cadence MHDP Audio driver
> + *
> */
> #include <linux/clk.h>
> #include <linux/reset.h>
> @@ -196,3 +199,100 @@ int cdns_mhdp_audio_config(struct cdns_mhdp_device *mhdp,
> return ret;
> }
> EXPORT_SYMBOL(cdns_mhdp_audio_config);
> +
> +static int audio_hw_params(struct device *dev, void *data,
> + struct hdmi_codec_daifmt *daifmt,
> + struct hdmi_codec_params *params)
> +{
> + struct cdns_mhdp_device *mhdp = dev_get_drvdata(dev);
> + struct audio_info audio = {
> + .sample_width = params->sample_width,
> + .sample_rate = params->sample_rate,
> + .channels = params->channels,
> + .connector_type = mhdp->connector.base.connector_type,
> + };
> + int ret;
> +
> + switch (daifmt->fmt) {
> + case HDMI_I2S:
> + audio.format = AFMT_I2S;
> + break;
> + case HDMI_SPDIF:
> + audio.format = AFMT_SPDIF_EXT;
> + break;
> + default:
> + DRM_DEV_ERROR(dev, "Invalid format %d\n", daifmt->fmt);
> + ret = -EINVAL;
> + goto out;
> + }
> +
> + ret = cdns_mhdp_audio_config(mhdp, &audio);
> + if (!ret)
> + mhdp->audio_info = audio;
> +
> +out:
> + return ret;
> +}
> +
> +static void audio_shutdown(struct device *dev, void *data)
> +{
> + struct cdns_mhdp_device *mhdp = dev_get_drvdata(dev);
> + int ret;
> +
> + ret = cdns_mhdp_audio_stop(mhdp, &mhdp->audio_info);
> + if (!ret)
> + mhdp->audio_info.format = AFMT_UNUSED;
> +}
> +
> +static int audio_digital_mute(struct device *dev, void *data,
> + bool enable)
> +{
> + struct cdns_mhdp_device *mhdp = dev_get_drvdata(dev);
> + int ret;
> +
> + ret = cdns_mhdp_audio_mute(mhdp, enable);
> +
> + return ret;
> +}
> +
> +static int audio_get_eld(struct device *dev, void *data,
> + u8 *buf, size_t len)
> +{
> + struct cdns_mhdp_device *mhdp = dev_get_drvdata(dev);
> +
> + memcpy(buf, mhdp->connector.base.eld,
> + min(sizeof(mhdp->connector.base.eld), len));
> +
> + return 0;
> +}
> +
> +static const struct hdmi_codec_ops audio_codec_ops = {
> + .hw_params = audio_hw_params,
> + .audio_shutdown = audio_shutdown,
> + .digital_mute = audio_digital_mute,
> + .get_eld = audio_get_eld,
> +};
> +
> +int cdns_mhdp_register_audio_driver(struct device *dev)
> +{
> + struct cdns_mhdp_device *mhdp = dev_get_drvdata(dev);
> + struct hdmi_codec_pdata codec_data = {
> + .i2s = 1,
> + .spdif = 1,
> + .ops = &audio_codec_ops,
> + .max_i2s_channels = 8,
> + };
> +
> + mhdp->audio_pdev = platform_device_register_data(
> + dev, HDMI_CODEC_DRV_NAME, 1,
> + &codec_data, sizeof(codec_data));
> +
> + return PTR_ERR_OR_ZERO(mhdp->audio_pdev);
> +}
> +
> +void cdns_mhdp_unregister_audio_driver(struct device *dev)
> +{
> + struct cdns_mhdp_device *mhdp = dev_get_drvdata(dev);
> +
> + platform_device_unregister(mhdp->audio_pdev);
> +}
> diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp-common.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp-common.c
> index efb39cf5f48b..9fd4546c6914 100644
> --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp-common.c
> +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp-common.c
> @@ -357,36 +357,6 @@ int cdns_mhdp_set_firmware_active(struct cdns_mhdp_device *mhdp, bool enable)
> }
> EXPORT_SYMBOL(cdns_mhdp_set_firmware_active);
>
> -int cdns_mhdp_set_host_cap(struct cdns_mhdp_device *mhdp, bool flip)
> -{
> - u8 msg[8];
> - int ret;
> -
> - msg[0] = drm_dp_link_rate_to_bw_code(mhdp->dp.rate);
> - msg[1] = mhdp->dp.num_lanes | SCRAMBLER_EN;
> - msg[2] = VOLTAGE_LEVEL_2;
> - msg[3] = PRE_EMPHASIS_LEVEL_3;
> - msg[4] = PTS1 | PTS2 | PTS3 | PTS4;
> - msg[5] = FAST_LT_NOT_SUPPORT;
> - msg[6] = flip ? LANE_MAPPING_FLIPPED : LANE_MAPPING_NORMAL;
> - msg[7] = ENHANCED;
> -
> - ret = cdns_mhdp_mailbox_send(mhdp, MB_MODULE_ID_DP_TX,
> - DPTX_SET_HOST_CAPABILITIES,
> - sizeof(msg), msg);
> - if (ret)
> - goto err_set_host_cap;
> -
> - ret = cdns_mhdp_reg_write(mhdp, DP_AUX_SWAP_INVERSION_CONTROL,
> - AUX_HOST_INVERT);
> -
> -err_set_host_cap:
> - if (ret)
> - DRM_DEV_ERROR(mhdp->dev, "set host cap failed: %d\n", ret);
> - return ret;
> -}
> -EXPORT_SYMBOL(cdns_mhdp_set_host_cap);
> -
> int cdns_mhdp_event_config(struct cdns_mhdp_device *mhdp)
> {
> u8 msg[5];
> @@ -698,3 +668,15 @@ int cdns_mhdp_config_video(struct cdns_mhdp_device *mhdp)
> return ret;
> }
> EXPORT_SYMBOL(cdns_mhdp_config_video);
> +
> +int cdns_phy_reg_write(struct cdns_mhdp_device *mhdp, u32 addr, u32 val)
> +{
> + return cdns_mhdp_reg_write(mhdp, ADDR_PHY_AFE + (addr << 2), val);
> +}
> +EXPORT_SYMBOL(cdns_phy_reg_write);
> +
> +u32 cdns_phy_reg_read(struct cdns_mhdp_device *mhdp, u32 addr)
> +{
> + return cdns_mhdp_reg_read(mhdp, ADDR_PHY_AFE + (addr << 2));
> +}
> +EXPORT_SYMBOL(cdns_phy_reg_read);
> diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp-common.h b/drivers/gpu/drm/bridge/cadence/cdns-mhdp-common.h
> index 1f093a2deaa7..b122bf5f0bdf 100644
> --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp-common.h
> +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp-common.h
> @@ -20,4 +20,7 @@ int cdns_mhdp_reg_write(struct cdns_mhdp_device *mhdp, u32 addr, u32 val);
> int cdns_mhdp_reg_write_bit(struct cdns_mhdp_device *mhdp, u16 addr,
> u8 start_bit, u8 bits_no, u32 val);
>
> +/* Audio */
> +int cdns_mhdp_register_audio_driver(struct device *dev);
> +void cdns_mhdp_unregister_audio_driver(struct device *dev);
> #endif
> diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp-dp.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp-dp.c
> index c8160f321aca..8fea072a5568 100644
> --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp-dp.c
> +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp-dp.c
> @@ -108,7 +108,9 @@ static int cdns_mhdp_training_start(struct cdns_mhdp_device *mhdp)
> if (ret)
> goto err_training_start;
>
> - if (event[1] & EQ_PHASE_FINISHED)
> + if (event[1] & CLK_RECOVERY_FAILED)
> + DRM_DEV_ERROR(mhdp->dev, "clock recovery failed\n");
> + else if (event[1] & EQ_PHASE_FINISHED)
> return 0;
> }
>
> @@ -172,3 +174,33 @@ int cdns_mhdp_train_link(struct cdns_mhdp_device *mhdp)
> return ret;
> }
> EXPORT_SYMBOL(cdns_mhdp_train_link);
> +
> +int cdns_mhdp_set_host_cap(struct cdns_mhdp_device *mhdp)
> +{
> + u8 msg[8];
> + int ret;
> +
> + msg[0] = drm_dp_link_rate_to_bw_code(mhdp->dp.rate);
> + msg[1] = mhdp->dp.num_lanes | SCRAMBLER_EN;
> + msg[2] = VOLTAGE_LEVEL_2;
> + msg[3] = PRE_EMPHASIS_LEVEL_3;
> + msg[4] = PTS1 | PTS2 | PTS3 | PTS4;
> + msg[5] = FAST_LT_NOT_SUPPORT;
> + msg[6] = mhdp->lane_mapping;
> + msg[7] = ENHANCED;
> +
> + ret = cdns_mhdp_mailbox_send(mhdp, MB_MODULE_ID_DP_TX,
> + DPTX_SET_HOST_CAPABILITIES,
> + sizeof(msg), msg);
> + if (ret)
> + goto err_set_host_cap;
> +
> + ret = cdns_mhdp_reg_write(mhdp, DP_AUX_SWAP_INVERSION_CONTROL,
> + AUX_HOST_INVERT);
> +
> +err_set_host_cap:
> + if (ret)
> + DRM_DEV_ERROR(mhdp->dev, "set host cap failed: %d\n", ret);
> + return ret;
> +}
> +EXPORT_SYMBOL(cdns_mhdp_set_host_cap);
> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c b/drivers/gpu/drm/rockchip/cdn-dp-core.c
> index 06fd82b217b6..d94d22650899 100644
> --- a/drivers/gpu/drm/rockchip/cdn-dp-core.c
> +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c
> @@ -423,7 +423,12 @@ static int cdn_dp_enable_phy(struct cdn_dp_device *dp, struct cdn_dp_port *port)
> }
>
> port->lanes = cdn_dp_get_port_lanes(port);
> - ret = cdns_mhdp_set_host_cap(&dp->mhdp, property.intval);
> +
> + if (property.intval)
> + dp->mhdp.lane_mapping = LANE_MAPPING_FLIPPED;
> + else
> + dp->mhdp.lane_mapping = LANE_MAPPING_NORMAL;
> + ret = cdns_mhdp_set_host_cap(&dp->mhdp);
> if (ret) {
> DRM_DEV_ERROR(dev, "set host capabilities failed: %d\n",
> ret);
> diff --git a/include/drm/bridge/cdns-mhdp.h b/include/drm/bridge/cdns-mhdp.h
> index c8170e6048f7..6ffb97e17fae 100644
> --- a/include/drm/bridge/cdns-mhdp.h
> +++ b/include/drm/bridge/cdns-mhdp.h
> @@ -14,6 +14,7 @@
>
> #define ADDR_IMEM 0x10000
> #define ADDR_DMEM 0x20000
> +#define ADDR_PHY_AFE 0x80000
>
> /* APB CFG addr */
> #define APB_CTRL 0
> @@ -81,6 +82,10 @@
> #define SOURCE_PIF_SW_RESET 0x30834
>
> /* bellow registers need access by mailbox */
> +
> +/* source phy comp */
> +#define LANES_CONFIG 0x0814
> +
> /* source car addr */
> #define SOURCE_HDTX_CAR 0x0900
> #define SOURCE_DPTX_CAR 0x0904
> @@ -424,6 +429,16 @@
> /* Reference cycles when using lane clock as reference */
> #define LANE_REF_CYC 0x8000
>
> +#define HOTPLUG_DEBOUNCE_MS 200
> +
> +#define IRQ_IN 0
> +#define IRQ_OUT 1
> +#define IRQ_NUM 2
> +
> +#define cdns_mhdp_plat_call(mhdp, operation) \
> + (!(mhdp) ? -ENODEV : (((mhdp)->plat_data && (mhdp)->plat_data->operation) ? \
> + (mhdp)->plat_data->operation(mhdp) : ENOIOCTLCMD))
> +
> enum voltage_swing_level {
> VOLTAGE_LEVEL_0,
> VOLTAGE_LEVEL_1,
> @@ -504,9 +519,29 @@ struct cdns_mhdp_connector {
> struct cdns_mhdp_bridge *bridge;
> };
>
> +struct cdns_plat_data {
> + /* Vendor PHY support */
> + int (*bind)(struct platform_device *pdev,
> + struct drm_encoder *encoder,
> + struct cdns_mhdp_device *mhdp);
> + void (*unbind)(struct device *dev);
> +
> + int (*phy_set)(struct cdns_mhdp_device *mhdp);
> + bool (*phy_video_valid)(struct cdns_mhdp_device *mhdp);
> + int (*firmware_init)(struct cdns_mhdp_device *mhdp);
> + void (*pclk_rate)(struct cdns_mhdp_device *mhdp);
> +
> + int (*power_on)(struct cdns_mhdp_device *mhdp);
> + int (*power_off)(struct cdns_mhdp_device *mhdp);
> +
> + char *plat_name;
> + int lane_mapping;
> +};
> +
> struct cdns_mhdp_device {
> struct device *dev;
> struct cdns_mhdp_connector connector;
> + struct cdns_mhdp_bridge bridge;
>
> void __iomem *regs_base;
> struct reset_control *spdif_rst;
> @@ -520,6 +555,11 @@ struct cdns_mhdp_device {
>
> unsigned int fw_version;
>
> + struct delayed_work hotplug_work;
> + u32 lane_mapping;
> + bool power_up;
> + struct mutex lock;
> + int irq[IRQ_NUM];
> union {
> struct _dp_data {
> u32 rate;
> @@ -528,6 +568,8 @@ struct cdns_mhdp_device {
> u8 dpcd[DP_RECEIVER_CAP_SIZE];
> } dp;
> };
> + const struct cdns_plat_data *plat_data;
> +
> };
>
> void cdns_mhdp_clock_reset(struct cdns_mhdp_device *mhdp);
> @@ -535,7 +577,7 @@ void cdns_mhdp_set_fw_clk(struct cdns_mhdp_device *mhdp, unsigned long clk);
> int cdns_mhdp_load_firmware(struct cdns_mhdp_device *mhdp, const u32 *i_mem,
> u32 i_size, const u32 *d_mem, u32 d_size);
> int cdns_mhdp_set_firmware_active(struct cdns_mhdp_device *mhdp, bool enable);
> -int cdns_mhdp_set_host_cap(struct cdns_mhdp_device *mhdp, bool flip);
> +int cdns_mhdp_set_host_cap(struct cdns_mhdp_device *mhdp);
> int cdns_mhdp_event_config(struct cdns_mhdp_device *mhdp);
> u32 cdns_mhdp_get_event(struct cdns_mhdp_device *mhdp);
> int cdns_mhdp_read_hpd(struct cdns_mhdp_device *mhdp);
> @@ -547,10 +589,18 @@ int cdns_mhdp_get_edid_block(void *mhdp, u8 *edid,
> int cdns_mhdp_train_link(struct cdns_mhdp_device *mhdp);
> int cdns_mhdp_set_video_status(struct cdns_mhdp_device *mhdp, int active);
> int cdns_mhdp_config_video(struct cdns_mhdp_device *mhdp);
> +bool cdns_mhdp_check_alive(struct cdns_mhdp_device *mhdp);
> int cdns_mhdp_audio_stop(struct cdns_mhdp_device *mhdp,
> struct audio_info *audio);
> int cdns_mhdp_audio_mute(struct cdns_mhdp_device *mhdp, bool enable);
> int cdns_mhdp_audio_config(struct cdns_mhdp_device *mhdp,
> struct audio_info *audio);
>
> +int cdns_phy_reg_write(struct cdns_mhdp_device *mhdp, u32 addr, u32 val);
> +u32 cdns_phy_reg_read(struct cdns_mhdp_device *mhdp, u32 addr);
> +
> +/* DP */
> +int cdns_dp_bind(struct platform_device *pdev, struct drm_encoder *encoder,
> + struct cdns_mhdp_device *mhdp);
> +void cdns_dp_unbind(struct device *dev);
> #endif /* CDNS_MHDP_H_ */
--
Regards,
Laurent Pinchart
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 00/10] spi: Adding support for Microchip Sparx5 SoC
From: Serge Semin @ 2020-06-02 23:44 UTC (permalink / raw)
To: Lars Povlsen
Cc: devicetree, linux-kernel, Serge Semin, linux-spi, SoC Team,
Mark Brown, Microchip Linux Driver Support, linux-arm-kernel
In-Reply-To: <87img9q3sb.fsf@soft-dev15.microsemi.net>
On Tue, Jun 02, 2020 at 10:18:28AM +0200, Lars Povlsen wrote:
>
> Serge Semin writes:
>
> > Hello Lars,
> >
> > On Wed, May 13, 2020 at 04:00:21PM +0200, Lars Povlsen wrote:
> >> This is an add-on series to the main SoC Sparx5 series
> >> (Message-ID: <20200513125532.24585-1-lars.povlsen@microchip.com>).
> >>
> >> The series add support for Sparx5 on top of the existing
> >> ocelot/jaguar2 spi driver.
> >>
> >> It spins off the existing support for the MSCC platforms into a
> >> separate driver, as adding new platforms from the MSCC/Microchip
> >> product lines will further complicate (clutter) the original driver.
> >>
> >> New YAML dt-bindings are provided for the resulting driver.
> >>
> >> It is expected that the DT patches are to be taken directly by the arm-soc
> >> maintainers.
> >
> > Regarding our cooperation. It can be implemented as follows. Since your patchset
> > is less cumbersome than mine and is more ready to be integrated into the generic DW
> > APB SSI code, it would be better to first make it through Mark', Andy' and my reviews
> > to be further merged into the kernel version of the driver. After that I'll have
> > my code altered so it could be applied on top of your patches. When everything
> > is done we'll have a more comprehensive DW APB SSI driver with poll-based
> > PIO operations support, new features like rx-delay, etc.
> >
>
> Hi Serge!
>
> I think I would be able to work on the SPI patches this week. Should I
> base it on the current spi-next or 5.7? Then address the comments and
> send out a new revision?
Finally I've done a part of review. It must be enough for v2. As Mark said the
new version is supposed to be based on the spi-next, since that branch's got
all recent DW APB SSI patches applied.
-Sergey
>
> Thanks for reaching out.
>
> ---Lars
>
> > Thank you one more time for the series you've shared with us. Let's see what can
> > be done to improve it...
> >
> > -Sergey
> >
> >>
> >> Lars Povlsen (10):
> >> spi: dw: Add support for polled operation via no IRQ specified in DT
> >> spi: dw: Add support for RX sample delay register
> >> spi: dw: Add support for client driver memory operations
> >> dt-bindings: spi: Add bindings for spi-dw-mchp
> >> spi: spi-dw-mmio: Spin off MSCC platforms into spi-dw-mchp
> >> dt-bindings: spi: spi-dw-mchp: Add Sparx5 support
> >> spi: spi-dw-mchp: Add Sparx5 support
> >> arm64: dts: sparx5: Add SPI controller
> >> arm64: dts: sparx5: Add spi-nor support
> >> arm64: dts: sparx5: Add spi-nand devices
> >>
> >> .../bindings/spi/mscc,ocelot-spi.yaml | 89 ++++
> >> .../bindings/spi/snps,dw-apb-ssi.txt | 7 +-
> >> MAINTAINERS | 2 +
> >> arch/arm64/boot/dts/microchip/sparx5.dtsi | 37 ++
> >> .../boot/dts/microchip/sparx5_pcb125.dts | 16 +
> >> .../boot/dts/microchip/sparx5_pcb134.dts | 22 +
> >> .../dts/microchip/sparx5_pcb134_board.dtsi | 9 +
> >> .../boot/dts/microchip/sparx5_pcb135.dts | 23 +
> >> .../dts/microchip/sparx5_pcb135_board.dtsi | 9 +
> >> arch/mips/configs/generic/board-ocelot.config | 2 +-
> >> drivers/spi/Kconfig | 7 +
> >> drivers/spi/Makefile | 1 +
> >> drivers/spi/spi-dw-mchp.c | 399 ++++++++++++++++++
> >> drivers/spi/spi-dw-mmio.c | 93 ----
> >> drivers/spi/spi-dw.c | 31 +-
> >> drivers/spi/spi-dw.h | 4 +
> >> 16 files changed, 644 insertions(+), 107 deletions(-)
> >> create mode 100644 Documentation/devicetree/bindings/spi/mscc,ocelot-spi.yaml
> >> create mode 100644 drivers/spi/spi-dw-mchp.c
> >>
> >> --
> >> 2.26.2
> >>
> >> _______________________________________________
> >> linux-arm-kernel mailing list
> >> linux-arm-kernel@lists.infradead.org
> >> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
> --
> Lars Povlsen,
> Microchip
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 7/7] dt-bindings: display: Document Cadence MHDP HDMI/DP bindings
From: Laurent Pinchart @ 2020-06-02 23:44 UTC (permalink / raw)
To: sandor.yu
Cc: jernej.skrabec, heiko, jonas, narmstrong, hjc, dri-devel,
linux-kernel, a.hajda, linux-imx, linux-rockchip, dkos,
linux-arm-kernel
In-Reply-To: <9fa979f1099f7c02fd746f25002d8a652253d70f.1590982881.git.Sandor.yu@nxp.com>
Hi Sandor,
Thank you for the patch.
On Mon, Jun 01, 2020 at 02:17:37PM +0800, sandor.yu@nxp.com wrote:
> From: Sandor Yu <Sandor.yu@nxp.com>
>
> Document the bindings used for the Cadence MHDP HDMI/DP bridge.
>
> Signed-off-by: Sandor Yu <Sandor.yu@nxp.com>
> ---
> .../bindings/display/bridge/cdns,mhdp.yaml | 46 +++++++++++++++
> .../devicetree/bindings/display/imx/mhdp.yaml | 59 +++++++++++++++++++
Please split the patch in two.
> 2 files changed, 105 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/display/bridge/cdns,mhdp.yaml
> create mode 100644 Documentation/devicetree/bindings/display/imx/mhdp.yaml
>
> diff --git a/Documentation/devicetree/bindings/display/bridge/cdns,mhdp.yaml b/Documentation/devicetree/bindings/display/bridge/cdns,mhdp.yaml
> new file mode 100644
> index 000000000000..aa23feba744a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/cdns,mhdp.yaml
> @@ -0,0 +1,46 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause))
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/bridge/cdns,mhdp.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Cadence MHDP TX Encoder
> +
> +maintainers:
> + - Sandor Yu <Sandoryu@nxp.com>
> +
> +description: |
> + Cadence MHDP Controller supports one or more of the protocols,
> + such as HDMI and DisplayPort.
> + Each protocol requires a different FW binaries.
> +
> + This document defines device tree properties for the Cadence MHDP Encoder
> + (CDNS MHDP TX). It doesn't constitue a device tree binding
> + specification by itself but is meant to be referenced by platform-specific
> + device tree bindings.
> +
> + When referenced from platform device tree bindings the properties defined in
> + this document are defined as follows. The platform device tree bindings are
> + responsible for defining whether each property is required or optional.
> +
> +properties:
> + reg:
> + maxItems: 1
> + description: Memory mapped base address and length of the MHDP TX registers.
> +
> + interrupts:
> + maxItems: 2
> +
> + interrupt-names:
> + - const: plug_in
> + description: Hotplug detect interrupter for cable plugin event.
> + - const: plug_out
> + description: Hotplug detect interrupter for cable plugout event.
Does the IP core really have two different interrupt lines, one for
hot-plug and one for hot-unplug ? That's a very unusual design.
> +
> + port:
> + type: object
> + description: |
> + The connectivity of the MHDP TX with the rest of the system is
> + expressed in using ports as specified in the device graph bindings defined
> + in Documentation/devicetree/bindings/graph.txt. The numbering of the ports
> + is platform-specific.
> diff --git a/Documentation/devicetree/bindings/display/imx/mhdp.yaml b/Documentation/devicetree/bindings/display/imx/mhdp.yaml
> new file mode 100644
> index 000000000000..17850cfd1cb1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/imx/mhdp.yaml
> @@ -0,0 +1,59 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/bridge/mhdp.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Cadence MHDP Encoder
> +
> +maintainers:
> + - Sandor Yu <Sandoryu@nxp.com>
> +
> +description: |
> + The MHDP transmitter is a Cadence HD Display TX controller IP
> + with a companion PHY IP.
> + The MHDP supports one or more of the protocols,
> + such as HDMI(1.4 & 2.0), DisplayPort(1.2).
> + switching between the two modes (HDMI and DisplayPort)
> + requires reloading the appropriate FW
Does the IP core integrated in the imx8mp SoCs (as that is what this
binding targets) support both HDMI and DP ? If not this should be
reworded to be more specific to the SoC.
> +
> + These DT bindings follow the Cadence MHDP TX bindings defined in
> + Documentation/devicetree/bindings/display/bridge/cdns,mhdp.yaml with the
> + following device-specific properties.
> +
> +Properties:
Have you tried validating this with make dt_binding_check ? See
Documentation/devicetree/writing-schema.rst for more information.
> + compatible:
> + enum:
> + - nxp,imx8mq-cdns-hdmi
> + - nxp,imx8mq-cdns-dp
> +
> + reg: See cdns,mhdp.yaml.
This isn't how bindings are referenced. You need to reference the parent
binding with $ref, either globally, or on an individual property basis.
> +
> + interrupts: See cdns,mhdp.yaml.
> +
> + interrupt-names: See cdns,mhdp.yaml.
That's it ? No clocks, no power domains, no resets, no PHYs (especially
given that you mention a PHY companion IP above) ?
> +
> + ports: See cdns,mhdp.yaml.
This isn't correct. Please soo of-graph.txt. If can have either one port
node, or one ports node that contains one of more port subnodes. In this
case you need at least two ports, one for the input to the HDMI encoder,
and one for the HDMI output. The latter should be connected to a DT node
representing the HDMI connector. Yuo can search for "hdmi-connector" in
the .dts files in the kernel for plenty of examples.
> +
> +Required:
> + - compatible
> + - reg
> + - interrupts
> + - interrupt-names
> + - ports
> +
> +Example:
> + - |
> + mhdp: mhdp@32c00000 {
> + compatible = "nxp,imx8mq-cdns-hdmi";
> + reg = <0x32c00000 0x100000>;
> + interrupts = <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-names = "plug_in", "plug_out";
> +
> + ports {
> + mhdp_in: endpoint {
> + remote-endpoint = <&dcss_out>;
> + };
> + };
> + };
--
Regards,
Laurent Pinchart
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v8 04/10] drm: bridge: dw_mipi_dsi: allow bridge daisy chaining
From: Laurent Pinchart @ 2020-06-02 23:51 UTC (permalink / raw)
To: Adrian Ratiu
Cc: devicetree, Jernej Skrabec, Heiko Stuebner, Jonas Karlman,
linux-kernel, dri-devel, Andrzej Hajda, linux-imx, linux-rockchip,
kernel, linux-stm32, linux-arm-kernel
In-Reply-To: <20200427081952.3536741-5-adrian.ratiu@collabora.com>
Hi Adrian,
Thank you for the patch.
On Mon, Apr 27, 2020 at 11:19:46AM +0300, Adrian Ratiu wrote:
> Up until now the assumption was that the synopsis dsi bridge will
> directly connect to an encoder provided by the platform driver, but
> the current practice for drivers is to leave the encoder empty via
> the simple encoder API and add their logic to their own drm_bridge.
>
> Thus we need an ablility to connect the DSI bridge to another bridge
> provided by the platform driver, so we extend the dw_mipi_dsi bind()
> API with a new "previous bridge" arg instead of just hardcoding NULL.
>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
> ---
> New in v8.
> ---
> drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 6 ++++--
> drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c | 2 +-
> include/drm/bridge/dw_mipi_dsi.h | 5 ++++-
> 3 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> index 16fd87055e7b7..140ff40fa1b62 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> @@ -1456,11 +1456,13 @@ EXPORT_SYMBOL_GPL(dw_mipi_dsi_remove);
> /*
> * Bind/unbind API, used from platforms based on the component framework.
> */
> -int dw_mipi_dsi_bind(struct dw_mipi_dsi *dsi, struct drm_encoder *encoder)
> +int dw_mipi_dsi_bind(struct dw_mipi_dsi *dsi,
> + struct drm_encoder *encoder,
> + struct drm_bridge *prev_bridge)
> {
> int ret;
>
> - ret = drm_bridge_attach(encoder, &dsi->bridge, NULL, 0);
> + ret = drm_bridge_attach(encoder, &dsi->bridge, prev_bridge, 0);
Please note that chaining of bridges doesn't work well if multiple
bridges in the chain try to create a connector. This is why a
DRM_BRIDGE_ATTACH_NO_CONNECTOR flag has been added, with a helper to
create a connector for a chain of bridges (drm_bridge_connector_init()).
This won't play well with the component framework. I would recommend
using the of_drm_find_bridge() instead in the rockchip driver, and
deprecating dw_mipi_dsi_bind().
> if (ret) {
> DRM_ERROR("Failed to initialize bridge with drm\n");
> return ret;
> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
> index 3feff0c45b3f7..83ef43be78135 100644
> --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
> @@ -929,7 +929,7 @@ static int dw_mipi_dsi_rockchip_bind(struct device *dev,
> return ret;
> }
>
> - ret = dw_mipi_dsi_bind(dsi->dmd, &dsi->encoder);
> + ret = dw_mipi_dsi_bind(dsi->dmd, &dsi->encoder, NULL);
> if (ret) {
> DRM_DEV_ERROR(dev, "Failed to bind: %d\n", ret);
> return ret;
> diff --git a/include/drm/bridge/dw_mipi_dsi.h b/include/drm/bridge/dw_mipi_dsi.h
> index b0e390b3288e8..699b3531f5b36 100644
> --- a/include/drm/bridge/dw_mipi_dsi.h
> +++ b/include/drm/bridge/dw_mipi_dsi.h
> @@ -14,6 +14,7 @@
> #include <drm/drm_modes.h>
>
> struct drm_display_mode;
> +struct drm_bridge;
> struct drm_encoder;
> struct dw_mipi_dsi;
> struct mipi_dsi_device;
> @@ -62,7 +63,9 @@ struct dw_mipi_dsi *dw_mipi_dsi_probe(struct platform_device *pdev,
> const struct dw_mipi_dsi_plat_data
> *plat_data);
> void dw_mipi_dsi_remove(struct dw_mipi_dsi *dsi);
> -int dw_mipi_dsi_bind(struct dw_mipi_dsi *dsi, struct drm_encoder *encoder);
> +int dw_mipi_dsi_bind(struct dw_mipi_dsi *dsi,
> + struct drm_encoder *encoder,
> + struct drm_bridge *prev_bridge);
> void dw_mipi_dsi_unbind(struct dw_mipi_dsi *dsi);
> void dw_mipi_dsi_set_slave(struct dw_mipi_dsi *dsi, struct dw_mipi_dsi *slave);
>
--
Regards,
Laurent Pinchart
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 3/4] ARM: dts: stm32: make hdmi-transmitter node compliant with DT bindings
From: Laurent Pinchart @ 2020-06-02 23:55 UTC (permalink / raw)
To: Ricardo Cañuelo
Cc: marex, devicetree, michal.simek, xuwei5, robh+dt, mcoquelin.stm32,
kernel, linux-arm-kernel
In-Reply-To: <20200601063308.13045-4-ricardo.canuelo@collabora.com>
Hi Ricardo,
Thank you for the patch.
On Mon, Jun 01, 2020 at 08:33:07AM +0200, Ricardo Cañuelo wrote:
> Reorder the I2C slave addresses of the hdmi-transmitter node and remove
> the adi,input-style and adi,input-justification properties to meet the
> adi,adv7513 binding requirements.
>
> Signed-off-by: Ricardo Cañuelo <ricardo.canuelo@collabora.com>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> arch/arm/boot/dts/stm32mp15xx-dhcor-avenger96.dtsi | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/boot/dts/stm32mp15xx-dhcor-avenger96.dtsi b/arch/arm/boot/dts/stm32mp15xx-dhcor-avenger96.dtsi
> index 930202742a3f..b67a21a8698a 100644
> --- a/arch/arm/boot/dts/stm32mp15xx-dhcor-avenger96.dtsi
> +++ b/arch/arm/boot/dts/stm32mp15xx-dhcor-avenger96.dtsi
> @@ -185,8 +185,8 @@
> &i2c4 {
> hdmi-transmitter@3d {
> compatible = "adi,adv7513";
> - reg = <0x3d>, <0x2d>, <0x4d>, <0x5d>;
> - reg-names = "main", "cec", "edid", "packet";
> + reg = <0x3d>, <0x4d>, <0x2d> , <0x5d>;
> + reg-names = "main", "edid", "cec", "packet";
> clocks = <&cec_clock>;
> clock-names = "cec";
>
> @@ -204,8 +204,6 @@
> adi,input-depth = <8>;
> adi,input-colorspace = "rgb";
> adi,input-clock = "1x";
> - adi,input-style = <1>;
> - adi,input-justification = "evenly";
>
> ports {
> #address-cells = <1>;
--
Regards,
Laurent Pinchart
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 4/4] dt-bindings: drm: bridge: adi,adv7511.txt: convert to yaml
From: Laurent Pinchart @ 2020-06-03 0:03 UTC (permalink / raw)
To: Ricardo Cañuelo
Cc: marex, devicetree, michal.simek, xuwei5, robh+dt, mcoquelin.stm32,
kernel, linux-arm-kernel
In-Reply-To: <20200601063308.13045-5-ricardo.canuelo@collabora.com>
Hi Ricardo,
Thank you for the patch.
On Mon, Jun 01, 2020 at 08:33:08AM +0200, Ricardo Cañuelo wrote:
> Convert the ADV7511/11w/13/33/35 DT bindings to json-schema. The
> original binding has been split into two files: adi,adv7511.yaml for
> ADV7511/11W/13 and adi,adv7533.yaml for ADV7533/35.
>
> Signed-off-by: Ricardo Cañuelo <ricardo.canuelo@collabora.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Great work :-)
> ---
> .../bindings/display/bridge/adi,adv7511.txt | 143 -----------
> .../bindings/display/bridge/adi,adv7511.yaml | 231 ++++++++++++++++++
> .../bindings/display/bridge/adi,adv7533.yaml | 175 +++++++++++++
> 3 files changed, 406 insertions(+), 143 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
> create mode 100644 Documentation/devicetree/bindings/display/bridge/adi,adv7511.yaml
> create mode 100644 Documentation/devicetree/bindings/display/bridge/adi,adv7533.yaml
>
> diff --git a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
> deleted file mode 100644
> index 659523f538bf..000000000000
> --- a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
> +++ /dev/null
> @@ -1,143 +0,0 @@
> -Analog Devices ADV7511(W)/13/33/35 HDMI Encoders
> -------------------------------------------------
> -
> -The ADV7511, ADV7511W, ADV7513, ADV7533 and ADV7535 are HDMI audio and video
> -transmitters compatible with HDMI 1.4 and DVI 1.0. They support color space
> -conversion, S/PDIF, CEC and HDCP. ADV7533/5 supports the DSI interface for input
> -pixels, while the others support RGB interface.
> -
> -Required properties:
> -
> -- compatible: Should be one of:
> - "adi,adv7511"
> - "adi,adv7511w"
> - "adi,adv7513"
> - "adi,adv7533"
> - "adi,adv7535"
> -
> -- reg: I2C slave addresses
> - The ADV7511 internal registers are split into four pages exposed through
> - different I2C addresses, creating four register maps. Each map has it own
> - I2C address and acts as a standard slave device on the I2C bus. The main
> - address is mandatory, others are optional and revert to defaults if not
> - specified.
> -
> -
> -The ADV7511 supports a large number of input data formats that differ by their
> -color depth, color format, clock mode, bit justification and random
> -arrangement of components on the data bus. The combination of the following
> -properties describe the input and map directly to the video input tables of the
> -ADV7511 datasheet that document all the supported combinations.
> -
> -- adi,input-depth: Number of bits per color component at the input (8, 10 or
> - 12).
> -- adi,input-colorspace: The input color space, one of "rgb", "yuv422" or
> - "yuv444".
> -- adi,input-clock: The input clock type, one of "1x" (one clock cycle per
> - pixel), "2x" (two clock cycles per pixel), "ddr" (one clock cycle per pixel,
> - data driven on both edges).
> -
> -The following input format properties are required except in "rgb 1x" and
> -"yuv444 1x" modes, in which case they must not be specified.
> -
> -- adi,input-style: The input components arrangement variant (1, 2 or 3), as
> - listed in the input format tables in the datasheet.
> -- adi,input-justification: The input bit justification ("left", "evenly",
> - "right").
> -
> -- avdd-supply: A 1.8V supply that powers up the AVDD pin on the chip.
> -- dvdd-supply: A 1.8V supply that powers up the DVDD pin on the chip.
> -- pvdd-supply: A 1.8V supply that powers up the PVDD pin on the chip.
> -- dvdd-3v-supply: A 3.3V supply that powers up the pin called DVDD_3V
> - on the chip.
> -- bgvdd-supply: A 1.8V supply that powers up the BGVDD pin. This is
> - needed only for ADV7511.
> -
> -The following properties are required for ADV7533 and ADV7535:
> -
> -- adi,dsi-lanes: Number of DSI data lanes connected to the DSI host. It should
> - be one of 1, 2, 3 or 4.
> -- a2vdd-supply: 1.8V supply that powers up the A2VDD pin on the chip.
> -- v3p3-supply: A 3.3V supply that powers up the V3P3 pin on the chip.
> -- v1p2-supply: A supply that powers up the V1P2 pin on the chip. It can be
> - either 1.2V or 1.8V for ADV7533 but only 1.8V for ADV7535.
> -
> -Optional properties:
> -
> -- interrupts: Specifier for the ADV7511 interrupt
> -- pd-gpios: Specifier for the GPIO connected to the power down signal
> -
> -- adi,clock-delay: Video data clock delay relative to the pixel clock, in ps
> - (-1200 ps .. 1600 ps). Defaults to no delay.
> -- adi,embedded-sync: The input uses synchronization signals embedded in the
> - data stream (similar to BT.656). Defaults to separate H/V synchronization
> - signals.
> -- adi,disable-timing-generator: Only for ADV7533 and ADV7535. Disables the
> - internal timing generator. The chip will rely on the sync signals in the
> - DSI data lanes, rather than generate its own timings for HDMI output.
> -- clocks: from common clock binding: reference to the CEC clock.
> -- clock-names: from common clock binding: must be "cec".
> -- reg-names : Names of maps with programmable addresses.
> - It can contain any map needing a non-default address.
> - Possible maps names are : "main", "edid", "cec", "packet"
> -
> -Required nodes:
> -
> -The ADV7511 has two video ports. Their connections are modelled using the OF
> -graph bindings specified in Documentation/devicetree/bindings/graph.txt.
> -
> -- Video port 0 for the RGB, YUV or DSI input. In the case of ADV7533/5, the
> - remote endpoint phandle should be a reference to a valid mipi_dsi_host device
> - node.
> -- Video port 1 for the HDMI output
> -- Audio port 2 for the HDMI audio input
> -
> -
> -Example
> --------
> -
> - adv7511w: hdmi@39 {
> - compatible = "adi,adv7511w";
> - /*
> - * The EDID page will be accessible on address 0x66 on the I2C
> - * bus. All other maps continue to use their default addresses.
> - */
> - reg = <0x39>, <0x66>;
> - reg-names = "main", "edid";
> - interrupt-parent = <&gpio3>;
> - interrupts = <29 IRQ_TYPE_EDGE_FALLING>;
> - clocks = <&cec_clock>;
> - clock-names = "cec";
> -
> - adi,input-depth = <8>;
> - adi,input-colorspace = "rgb";
> - adi,input-clock = "1x";
> - adi,input-style = <1>;
> - adi,input-justification = "evenly";
> -
> - ports {
> - #address-cells = <1>;
> - #size-cells = <0>;
> -
> - port@0 {
> - reg = <0>;
> - adv7511w_in: endpoint {
> - remote-endpoint = <&dpi_out>;
> - };
> - };
> -
> - port@1 {
> - reg = <1>;
> - adv7511_out: endpoint {
> - remote-endpoint = <&hdmi_connector_in>;
> - };
> - };
> -
> - port@2 {
> - reg = <2>;
> - codec_endpoint: endpoint {
> - remote-endpoint = <&i2s0_cpu_endpoint>;
> - };
> - };
> - };
> - };
> diff --git a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.yaml b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.yaml
> new file mode 100644
> index 000000000000..71b344e812dd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.yaml
> @@ -0,0 +1,231 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/bridge/adi,adv7511.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices ADV7511/11W/13 HDMI Encoders
> +
> +maintainers:
> + - Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> +
> +description: |
> + The ADV7511, ADV7511W and ADV7513 are HDMI audio and video
> + transmitters compatible with HDMI 1.4 and DVI 1.0. They support color
> + space conversion, S/PDIF, CEC and HDCP. The transmitter input is
> + parallel RGB or YUV data.
> +
> +properties:
> + compatible:
> + enum:
> + - adi,adv7511
> + - adi,adv7511w
> + - adi,adv7513
> +
> + reg:
> + description: |
> + I2C slave addresses.
> +
> + The ADV7511/11W/13 internal registers are split into four pages
> + exposed through different I2C addresses, creating four register
> + maps. Each map has it own I2C address and acts as a standard slave
> + device on the I2C bus. The main address is mandatory, others are
> + optional and revert to defaults if not specified.
> + minItems: 1
> + maxItems: 4
> +
> + reg-names:
> + description:
> + Names of maps with programmable addresses. It can contain any map
> + needing a non-default address.
> + minItems: 1
> + items:
> + - const: main
> + - const: edid
> + - const: cec
> + - const: packet
> +
> + clocks:
> + description: Reference to the CEC clock.
> + maxItems: 1
> +
> + clock-names:
> + const: cec
> +
> + interrupts:
> + maxItems: 1
> +
> + pd-gpios:
> + description: GPIO connected to the power down signal.
> + maxItems: 1
> +
> + avdd-supply:
> + description: A 1.8V supply that powers up the AVDD pin.
> +
> + dvdd-supply:
> + description: A 1.8V supply that powers up the DVDD pin.
> +
> + pvdd-supply:
> + description: A 1.8V supply that powers up the PVDD pin.
> +
> + dvdd-3v-supply:
> + description: A 3.3V supply that powers up the DVDD_3V pin.
> +
> + bgvdd-supply:
> + description: A 1.8V supply that powers up the BGVDD pin.
> +
> + adi,input-depth:
> + description: Number of bits per color component at the input.
> + allOf:
> + - $ref: /schemas/types.yaml#/definitions/uint32
> + - enum: [ 8, 10, 12 ]
> +
> + adi,input-colorspace:
> + description: Input color space.
> + enum: [ rgb, yuv422, yuv444 ]
> +
> + adi,input-clock:
> + description: |
> + Input clock type.
> + "1x": one clock cycle per pixel
> + "2x": two clock cycles per pixel
> + "dd": one clock cycle per pixel, data driven on both edges
> + enum: [ 1x, 2x, dd ]
> +
> + adi,clock-delay:
> + description:
> + Video data clock delay relative to the pixel clock, in ps
> + (-1200ps .. 1600 ps).
> + $ref: /schemas/types.yaml#/definitions/uint32
> + default: 0
> +
> + adi,embedded-sync:
> + description:
> + If defined, the input uses synchronization signals embedded in the
> + data stream (similar to BT.656).
> + type: boolean
> +
> + adi,input-style:
> + description:
> + Input components arrangement variant as listed in the input
> + format tables in the datasheet.
> + $ref: /schemas/types.yaml#/definitions/uint32
> + enum: [ 1, 2, 3 ]
> +
> + adi,input-justification:
> + description: Input bit justification.
> + enum: [ left, evenly, right ]
> +
> + ports:
> + description:
> + The ADV7511(W)/13 has two video ports and one audio port. This node
> + models their connections as documented in
> + Documentation/devicetree/bindings/media/video-interfaces.txt
> + Documentation/devicetree/bindings/graph.txt
> + type: object
> + properties:
> + port@0:
> + description: Video port for the RGB or YUV input.
> + type: object
> +
> + port@1:
> + description: Video port for the HDMI output.
> + type: object
> +
> + port@2:
> + description: Audio port for the HDMI output.
> + type: object
> +
> +# adi,input-colorspace and adi,input-clock are required except in
> +# "rgb 1x" and "yuv444 1x" modes, in which case they must not be
> +# specified.
> +if:
> + not:
> + properties:
> + adi,input-colorspace:
> + contains:
> + enum: [ rgb, yuv444 ]
> + adi,input-clock:
> + contains:
> + const: 1x
> +
> +then:
> + required:
> + - adi,input-style
> + - adi,input-justification
> +
> +else:
> + properties:
> + adi,input-style: false
> + adi,input-justification: false
> +
> +
> +required:
> + - compatible
> + - reg
> + - ports
> + - adi,input-depth
> + - adi,input-colorspace
> + - adi,input-clock
> + - avdd-supply
> + - dvdd-supply
> + - pvdd-supply
> + - dvdd-3v-supply
> + - bgvdd-supply
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/irq.h>
> +
> + adv7511w: hdmi@39 {
> + compatible = "adi,adv7511w";
> + /*
> + * The EDID page will be accessible on address 0x66 on the I2C
> + * bus. All other maps continue to use their default addresses.
> + */
> + reg = <0x39>, <0x66>;
> + reg-names = "main", "edid";
> + interrupt-parent = <&gpio3>;
> + interrupts = <29 IRQ_TYPE_EDGE_FALLING>;
> + clocks = <&cec_clock>;
> + clock-names = "cec";
> + avdd-supply = <&v1v8>;
> + dvdd-supply = <&v1v8>;
> + pvdd-supply = <&v1v8>;
> + dvdd-3v-supply = <&v3v3>;
> + bgvdd-supply = <&v1v8>;
> +
> + adi,input-depth = <8>;
> + adi,input-colorspace = "yuv422";
> + adi,input-clock = "1x";
> +
> + adi,input-style = <3>;
> + adi,input-justification = "right";
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> + adv7511w_in: endpoint {
> + remote-endpoint = <&dpi_out>;
> + };
> + };
> +
> + port@1 {
> + reg = <1>;
> + adv7511_out: endpoint {
> + remote-endpoint = <&hdmi_connector_in>;
> + };
> + };
> +
> + port@2 {
> + reg = <2>;
> + codec_endpoint: endpoint {
> + remote-endpoint = <&i2s0_cpu_endpoint>;
> + };
> + };
> + };
> + };
> +
> +...
> diff --git a/Documentation/devicetree/bindings/display/bridge/adi,adv7533.yaml b/Documentation/devicetree/bindings/display/bridge/adi,adv7533.yaml
> new file mode 100644
> index 000000000000..18761f49e5fe
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/adi,adv7533.yaml
> @@ -0,0 +1,175 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/bridge/adi,adv7533.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices ADV7533/35 HDMI Encoders
> +
> +maintainers:
> + - Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> +
> +description: |
> + The ADV7533 and ADV7535 are HDMI audio and video transmitters
> + compatible with HDMI 1.4 and DVI 1.0. They support color space
> + conversion, S/PDIF, CEC and HDCP. The transmitter input is MIPI DSI.
> +
> +properties:
> + compatible:
> + enum:
> + - adi,adv7533
> + - adi,adv7535
> +
> + reg:
> + description: |
> + I2C slave addresses.
> +
> + The ADV7533/35 internal registers are split into four pages
> + exposed through different I2C addresses, creating four register
> + maps. Each map has it own I2C address and acts as a standard slave
> + device on the I2C bus. The main address is mandatory, others are
> + optional and revert to defaults if not specified.
> + minItems: 1
> + maxItems: 4
> +
> + reg-names:
> + description:
> + Names of maps with programmable addresses. It can contain any map
> + needing a non-default address.
> + minItems: 1
> + items:
> + - const: main
> + - const: edid
> + - const: cec
> + - const: packet
> +
> + clocks:
> + description: Reference to the CEC clock.
> + maxItems: 1
> +
> + clock-names:
> + const: cec
> +
> + interrupts:
> + maxItems: 1
> +
> + pd-gpios:
> + description: GPIO connected to the power down signal.
> + maxItems: 1
> +
> + avdd-supply:
> + description: A 1.8V supply that powers up the AVDD pin.
> +
> + dvdd-supply:
> + description: A 1.8V supply that powers up the DVDD pin.
> +
> + pvdd-supply:
> + description: A 1.8V supply that powers up the PVDD pin.
> +
> + a2vdd-supply:
> + description: A 1.8V supply that powers up the A2VDD pin.
> +
> + v3p3-supply:
> + description: A 3.3V supply that powers up the V3P3 pin.
> +
> + v1p2-supply:
> + description:
> + A supply that powers up the V1P2 pin. It can be either 1.2V
> + or 1.8V for ADV7533 but only 1.8V for ADV7535.
> +
> + adi,disable-timing-generator:
> + description:
> + Disables the internal timing generator. The chip will rely on the
> + sync signals in the DSI data lanes, rather than generating its own
> + timings for HDMI output.
> + type: boolean
> +
> + adi,dsi-lanes:
> + description: Number of DSI data lanes connected to the DSI host.
> + $ref: /schemas/types.yaml#/definitions/uint32
> + enum: [ 1, 2, 3, 4 ]
> +
> + ports:
> + description:
> + The ADV7533/35 has two video ports and one audio port. This node
> + models their connections as documented in
> + Documentation/devicetree/bindings/media/video-interfaces.txt
> + Documentation/devicetree/bindings/graph.txt
> + type: object
> + properties:
> + port@0:
> + description:
> + Video port for the DSI input. The remote endpoint phandle
> + should be a reference to a valid mipi_dsi_host_device.
> + type: object
> +
> + port@1:
> + description: Video port for the HDMI output.
> + type: object
> +
> + port@2:
> + description: Audio port for the HDMI output.
> + type: object
> +
> +required:
> + - compatible
> + - reg
> + - ports
> + - adi,dsi-lanes
> + - avdd-supply
> + - dvdd-supply
> + - pvdd-supply
> + - a2vdd-supply
> + - v3p3-supply
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/irq.h>
> +
> + adv7533: hdmi@39 {
> + compatible = "adi,adv7533";
> + /*
> + * The EDID page will be accessible on address 0x66 on the I2C
> + * bus. All other maps continue to use their default addresses.
> + */
> + reg = <0x39>, <0x66>;
> + reg-names = "main", "edid";
> + interrupt-parent = <&gpio3>;
> + interrupts = <29 IRQ_TYPE_EDGE_FALLING>;
> + clocks = <&cec_clock>;
> + clock-names = "cec";
> + adi,dsi-lanes = <4>;
> + avdd-supply = <&v1v8>;
> + dvdd-supply = <&v1v8>;
> + pvdd-supply = <&v1v8>;
> + a2vdd-supply = <&v1v8>;
> + v3p3-supply = <&v3v3>;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> + adv7533_in: endpoint {
> + remote-endpoint = <&dsi_out>;
> + };
> + };
> +
> + port@1 {
> + reg = <1>;
> + adv7533_out: endpoint {
> + remote-endpoint = <&hdmi_connector_in>;
> + };
> + };
> +
> + port@2 {
> + reg = <2>;
> + codec_endpoint: endpoint {
> + remote-endpoint = <&i2s0_cpu_endpoint>;
> + };
> + };
> + };
> + };
> +
> +...
> --
> 2.18.0
>
--
Regards,
Laurent Pinchart
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] pinctrl: sirf: Add missing put_device() call in sirfsoc_gpio_probe()
From: yukuai (C) @ 2020-06-03 1:12 UTC (permalink / raw)
To: Markus Elfring, linux-arm-kernel, linux-gpio
Cc: Barry Song, kernel-janitors, linux-kernel, Yi Zhang
In-Reply-To: <01abac73-2107-daf2-d7bd-bef9d73d554a@web.de>
On 2020/6/3 2:56, Markus Elfring wrote:
>> in sirfsoc_gpio_probe(), if of_find_device_by_node() succeed,
>> put_device() is missing in the error handling patch.
>
> How do you think about another wording variant?
>
> A coccicheck run provided information like the following.
>
> drivers/pinctrl/sirf/pinctrl-sirf.c:798:2-8: ERROR: missing put_device;
> call of_find_device_by_node on line 792, but without a corresponding
> object release within this function.
>
> Generated by: scripts/coccinelle/free/put_device.cocci
>
> Thus add a jump target to fix the exception handling for this
> function implementation.
>
>
> Would you like to add the tag “Fixes” to the commit message?
>
Will do, thanks for your advise!
Yu Kuai
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 1/2] media: dt-bindings: media: xilinx: Add Xilinx UHD-SDI Receiver Subsystem
From: Laurent Pinchart @ 2020-06-03 1:13 UTC (permalink / raw)
To: Vishal Sagar
Cc: mark.rutland@arm.com, devicetree@vger.kernel.org, Dinesh Kumar,
Hyun Kwon, Sandip Kothari, linux-kernel@vger.kernel.org,
robh+dt@kernel.org, Michal Simek, Joe Perches,
hans.verkuil@cisco.com, mchehab@kernel.org,
linux-arm-kernel@lists.infradead.org, linux-media@vger.kernel.org
In-Reply-To: <DM6PR02MB6876116CECBA49741FF57E79A78A0@DM6PR02MB6876.namprd02.prod.outlook.com>
Hi Vishal,
On Mon, Jun 01, 2020 at 03:14:52PM +0000, Vishal Sagar wrote:
> On Wednesday, May 6, 2020 6:32 PM, Laurent Pinchart wrote:
> > On Wed, Apr 29, 2020 at 07:47:03PM +0530, Vishal Sagar wrote:
> > > Add bindings documentation for Xilinx UHD-SDI Receiver Subsystem.
> > >
> > > The Xilinx UHD-SDI Receiver Subsystem consists of SMPTE UHD-SDI (RX) IP
> > > core, an SDI RX to Video Bridge IP core to convert SDI video to native
> > > video and a Video In to AXI4-Stream IP core to convert native video to
> > > AXI4-Stream.
> > >
> > > Signed-off-by: Vishal Sagar <vishal.sagar@xilinx.com>
> > > ---
> > > v2
> > > - Removed references to xlnx,video*
> > > - Fixed as per Sakari Ailus and Rob Herring's comments
> > > - Converted to yaml format
> > >
> > > .../bindings/media/xilinx/xlnx,sdirxss.yaml | 132 ++++++++++++++++++
> > > 1 file changed, 132 insertions(+)
> > > create mode 100644 Documentation/devicetree/bindings/media/xilinx/xlnx,sdirxss.yaml
> > >
> > > diff --git
> > a/Documentation/devicetree/bindings/media/xilinx/xlnx,sdirxss.yaml
> > b/Documentation/devicetree/bindings/media/xilinx/xlnx,sdirxss.yaml
> > > new file mode 100644
> > > index 000000000000..9133ad19df55
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/media/xilinx/xlnx,sdirxss.yaml
> > > @@ -0,0 +1,132 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/media/xilinx/xlnx,sdirxss.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +
> > > +title: Xilinx SMPTE UHD-SDI Receiver Subsystem
> > > +
> > > +maintainers:
> > > + - Vishal Sagar <vishal.sagar@xilinx.com>
> > > +
> > > +description: |
> > > + The SMPTE UHD-SDI Receiver (RX) Subsystem allows you to quickly create systems
> > > + based on SMPTE SDI protocols. It receives unaligned native SDI streams from
> > > + the SDI GT PHY and outputs an AXI4-Stream video stream, native video, or
> > > + native SDI using Xilinx transceivers as the physical layer.
> > > +
> > > + The subsystem consists of
> > > + 1 - SMPTE UHD-SDI Rx
> > > + 2 - SDI Rx to Native Video Bridge
> > > + 3 - Video In to AXI4-Stream Bridge
> > > +
> > > + The subsystem can capture SDI streams in upto 12G mode 8 data streams and output
> >
> > s/upto/up to/
>
> I will fix this in next version.
>
> > > + a dual pixel per clock RGB/YUV444,422/420 10/12 bits per component AXI4-Stream.
> > > +
> > > +properties:
> > > + compatible:
> > > + items:
> > > + - enum:
> > > + - xlnx,v-smpte-uhdsdi-rx-ss-2.0
> > > +
> > > + reg:
> > > + maxItems: 1
> > > +
> > > + interrupts:
> > > + maxItems: 1
> > > +
> > > + clocks:
> > > + description: List of clock specifiers
> > > + items:
> > > + - description: AXI4-Lite clock
> > > + - description: SMPTE UHD-SDI Rx core clock
> > > + - description: Video clock
> > > +
> > > + clock-names:
> > > + items:
> > > + - const: s_axi_aclk
> > > + - const: sdi_rx_clk
> > > + - const: video_out_clk
> > > +
> > > + xlnx,bpp:
> > > + description: Bits per pixel supported. Can be 10 or 12 bits per pixel only.
> > > + allOf:
> > > + - $ref: "/schemas/types.yaml#/definitions/uint32"
> > > + - enum: [10, 12]
> >
> > I don't see this as a design parameter in the documentation (pg290,
> > v2.0). What does it correspond to ? All the BPC mentions in the
> > documentation always state that 10-bit is the only supported value.
>
> The new version of IP being released will have 10 and 12 bit support. It is already in the Xilinx linux-xlnx repo.
> I will rename this to "xlnx,bpc" instead of "xlnx,bpp" to refer to bits per component.
Is the documentation for the new IP core version available ? Should this
property only be allowed for the new version, given that in v2.0 the BPC
is fixed to 10 ?
> > > +
> > > + xlnx,line-rate:
> > > + description: |
> > > + The maximum mode supported by the design. Possible values are as below
> > > + 12G_SDI_8DS - 12G mode with 8 data streams
> > > + 6G_SDI - 6G mode
> > > + 3G_SDI - 3G mode
> > > + enum:
> > > + - 12G_SDI_8DS
> > > + - 6G_SDI
> > > + - 3G_SDI
> >
> > How about making this an integer property, with #define in
> > include/dt-bindings/media/xilinx-sdi.h ? As far as I understand, the SDI
> > TX subsystem has the same parameter, so the #define could be shared
> > between the two.
>
> Yes that is ok with me. I will add this in the next version.
>
> > > +
> > > + xlnx,include-edh:
> > > + type: boolean
> > > + description: |
> > > + This is present when the Error Detection and Handling processor is
> > > + enabled in design.
> > > +
> > > + ports:
> > > + type: object
> > > + description: |
> > > + Generally the SDI port is connected to a device like SDI Broadcast camera
> > > + which is independently controlled. Hence port@0 is a source port which can be
> > > + connected to downstream IP which can work with AXI4 Stream data.
> >
> > We should still have an input port. It can be connected to a DT node for
> > a physical SDI connector, or any other component in the platform (I
> > expect the former to be the common case). There are DT bindings for
> > connectors in Documentation/devicetree/bindings/display/connector/, we
> > should add one for SDI.
>
> Yes the input port is a physical SDI connector connected to an equipment like broadcast camera.
> But the camera/equipment can't be controlled by the V4L2 pipeline and SDI protocol is unidirectional.
>
> If we add another dt node, then I think another dummy v4l subdev driver will need to implemented and loaded
> to complete the pipe as Xilinx Video driver will need it.
We don't necessarily need a driver for the connector (although it may be
a good idea to do so, but that's a separate question). The sdi-rx driver
could handle the SDI connector DT node by parsing its properties
manually (assuming it would contain properties that need to be parsed).
> Could you please share the reason to have this input port in the SDI Rx driver?
We try to make sure the whole pipeline is modelled in DT. This applies
to both V4L2 and DRM/KMS. While the connector doesn't need to be
controlled by software (it's just a connector), it may still have
properties that matter from a software point of view. For instance the
label property can be used to specify how the connector is labeled on
the board or on the device's case, allowing applications to display the
correct labels to the users. Another use case related to the 4-pin
mini-DIN connectors typically used for S-Video. On some devices, they
are also used for composite video, with multiple video sources connected
to the same mini-DIN connector with a special cable. Kernel drivers need
to know how signals are routed, and DT nodes help there.
> > > + properties:
> > > + port@0:
> > > + type: object
> > > + description: Source port
> > > + properties:
> > > + reg:
> > > + const: 0
> > > + endpoint:
> > > + type: object
> > > + properties:
> > > + remote-endpoint: true
> > > + required:
> > > + - remote-endpoint
> > > + additionalProperties: false
> > > + additionalProperties: false
> > > +
> > > +required:
> > > + - compatible
> > > + - reg
> > > + - interrupts
> > > + - clocks
> > > + - clock-names
> > > + - xlnx,line-rate
> > > + - xlnx,bpp
> > > + - ports
> > > +
> > > +additionalProperties: false
> > > +
> > > +examples:
> > > + - |
> > > + uhdsdirxss: v-smpte-uhdsdi-rxss@80000000 {
I forgot to mention, you can drop the label as it's not used.
> > > + compatible = "xlnx,v-smpte-uhdsdi-rx-ss-2.0";
> > > + interrupt-parent = <&gic>;
> > > + interrupts = <0 89 4>;
> > > + reg = <0x0 0x80000000 0x0 0x10000>;
> > > + xlnx,include-edh;
> > > + xlnx,line-rate = "12G_SDI_8DS";
> > > + clocks = <&clk_1>, <&si570_1>, <&clk_2>;
> > > + clock-names = "s_axi_aclk", "sdi_rx_clk", "video_out_clk";
> > > + xlnx,bpp = <10>;
And I would group the xlnx,* properties after the standard properties.
> > > +
> > > + ports {
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > + port@0 {
> > > + reg = <0>;
> > > + sdirx_out: endpoint {
> > > + remote-endpoint = <&vcap_sdirx_in>;
> > > + };
> > > + };
> > > + };
> > > + };
--
Regards,
Laurent Pinchart
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH V2] pinctrl: sirf: add missing put_device() call in sirfsoc_gpio_probe()
From: yu kuai @ 2020-06-03 1:35 UTC (permalink / raw)
To: linus.walleij, baohua, yuping.luo, Markus.Elfring
Cc: linux-gpio, yi.zhang, linux-kernel, linux-arm-kernel, yukuai3
A coccicheck run provided information like the following:
drivers/pinctrl/sirf/pinctrl-sirf.c:798:2-8: ERROR: missing put_device;
call of_find_device_by_node on line 792, but without a corresponding
object release within this function.
Generated by: scripts/coccinelle/free/put_device.cocci
Thus add a jump target to fix the exception handling for this
function implementation.
Fixes: 5130216265f6 ("PINCTRL: SiRF: add GPIO and GPIO irq support in CSR SiRFprimaII")
Signed-off-by: yu kuai <yukuai3@huawei.com>
---
drivers/pinctrl/sirf/pinctrl-sirf.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/drivers/pinctrl/sirf/pinctrl-sirf.c b/drivers/pinctrl/sirf/pinctrl-sirf.c
index 1ebcb957c654..63a287d5795f 100644
--- a/drivers/pinctrl/sirf/pinctrl-sirf.c
+++ b/drivers/pinctrl/sirf/pinctrl-sirf.c
@@ -794,13 +794,17 @@ static int sirfsoc_gpio_probe(struct device_node *np)
return -ENODEV;
sgpio = devm_kzalloc(&pdev->dev, sizeof(*sgpio), GFP_KERNEL);
- if (!sgpio)
- return -ENOMEM;
+ if (!sgpio) {
+ err = -ENOMEM;
+ goto out_put_device;
+ }
spin_lock_init(&sgpio->lock);
regs = of_iomap(np, 0);
- if (!regs)
- return -ENOMEM;
+ if (!regs) {
+ err = -ENOMEM;
+ goto out_put_device;
+ }
sgpio->chip.gc.request = sirfsoc_gpio_request;
sgpio->chip.gc.free = sirfsoc_gpio_free;
@@ -824,8 +828,10 @@ static int sirfsoc_gpio_probe(struct device_node *np)
girq->parents = devm_kcalloc(&pdev->dev, SIRFSOC_GPIO_NO_OF_BANKS,
sizeof(*girq->parents),
GFP_KERNEL);
- if (!girq->parents)
- return -ENOMEM;
+ if (!girq->parents) {
+ err = -ENOMEM;
+ goto out_put_device;
+ }
for (i = 0; i < SIRFSOC_GPIO_NO_OF_BANKS; i++) {
bank = &sgpio->sgpio_bank[i];
spin_lock_init(&bank->lock);
@@ -868,6 +874,8 @@ static int sirfsoc_gpio_probe(struct device_node *np)
gpiochip_remove(&sgpio->chip.gc);
out:
iounmap(regs);
+out_put_device:
+ put_device(&pdev->dev);
return err;
}
--
2.25.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH V2] pinctrl: sirf: add missing put_device() call in sirfsoc_gpio_probe()
From: yukuai (C) @ 2020-06-03 1:39 UTC (permalink / raw)
To: linus.walleij, baohua, yuping.luo, Markus.Elfring
Cc: linux-gpio, linux-kernel, linux-arm-kernel, yi.zhang
In-Reply-To: <20200603013532.755220-1-yukuai3@huawei.com>
On 2020/6/3 9:35, yu kuai wrote:
> A coccicheck run provided information like the following:
>
> drivers/pinctrl/sirf/pinctrl-sirf.c:798:2-8: ERROR: missing put_device;
> call of_find_device_by_node on line 792, but without a corresponding
> object release within this function.
>
> Generated by: scripts/coccinelle/free/put_device.cocci
>
> Thus add a jump target to fix the exception handling for this
> function implementation.
>
> Fixes: 5130216265f6 ("PINCTRL: SiRF: add GPIO and GPIO irq support in CSR SiRFprimaII")
> Signed-off-by: yu kuai <yukuai3@huawei.com>
> ---
> drivers/pinctrl/sirf/pinctrl-sirf.c | 20 ++++++++++++++------
> 1 file changed, 14 insertions(+), 6 deletions(-)
Sorry about the missing change log:
Changes in V2:
change the variant of commit message suggested by Markus.
Best Regards,
Yu Kuai
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH 0/3] Convert i.MX/MXS I2C/LPI2C binding doc to json-schema
From: Anson Huang @ 2020-06-03 1:58 UTC (permalink / raw)
To: aisheng.dong, robh+dt, shawnguo, s.hauer, kernel, festevam, linux,
pandy.gao, wolfram, linux-i2c, devicetree, linux-arm-kernel,
linux-kernel
Cc: Linux-imx
Coverts i.MX/MXS I2C.LPI2C binding doc to json-schema, some examples are
too old, update them based on latest DT file, also add more compatible
based on supported SoCs.
Anson Huang (3):
dt-bindings: i2c: Convert imx lpi2c to json-schema
dt-bindings: i2c: Convert mxs i2c to json-schema
dt-bindings: i2c: Convert imx i2c to json-schema
.../devicetree/bindings/i2c/i2c-imx-lpi2c.txt | 20 ----
.../devicetree/bindings/i2c/i2c-imx-lpi2c.yaml | 45 ++++++++
Documentation/devicetree/bindings/i2c/i2c-imx.txt | 49 ---------
Documentation/devicetree/bindings/i2c/i2c-imx.yaml | 118 +++++++++++++++++++++
Documentation/devicetree/bindings/i2c/i2c-mxs.txt | 25 -----
Documentation/devicetree/bindings/i2c/i2c-mxs.yaml | 55 ++++++++++
6 files changed, 218 insertions(+), 94 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/i2c/i2c-imx-lpi2c.txt
create mode 100644 Documentation/devicetree/bindings/i2c/i2c-imx-lpi2c.yaml
delete mode 100644 Documentation/devicetree/bindings/i2c/i2c-imx.txt
create mode 100644 Documentation/devicetree/bindings/i2c/i2c-imx.yaml
delete mode 100644 Documentation/devicetree/bindings/i2c/i2c-mxs.txt
create mode 100644 Documentation/devicetree/bindings/i2c/i2c-mxs.yaml
--
2.7.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH 1/3] dt-bindings: i2c: Convert imx lpi2c to json-schema
From: Anson Huang @ 2020-06-03 1:58 UTC (permalink / raw)
To: aisheng.dong, robh+dt, shawnguo, s.hauer, kernel, festevam, linux,
pandy.gao, wolfram, linux-i2c, devicetree, linux-arm-kernel,
linux-kernel
Cc: Linux-imx
In-Reply-To: <1591149535-342-1-git-send-email-Anson.Huang@nxp.com>
Convert the i.MX LPI2C binding to DT schema format using json-schema
Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
.../devicetree/bindings/i2c/i2c-imx-lpi2c.txt | 20 ----------
.../devicetree/bindings/i2c/i2c-imx-lpi2c.yaml | 45 ++++++++++++++++++++++
2 files changed, 45 insertions(+), 20 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/i2c/i2c-imx-lpi2c.txt
create mode 100644 Documentation/devicetree/bindings/i2c/i2c-imx-lpi2c.yaml
diff --git a/Documentation/devicetree/bindings/i2c/i2c-imx-lpi2c.txt b/Documentation/devicetree/bindings/i2c/i2c-imx-lpi2c.txt
deleted file mode 100644
index f0c072f..0000000
--- a/Documentation/devicetree/bindings/i2c/i2c-imx-lpi2c.txt
+++ /dev/null
@@ -1,20 +0,0 @@
-* Freescale Low Power Inter IC (LPI2C) for i.MX
-
-Required properties:
-- compatible :
- - "fsl,imx7ulp-lpi2c" for LPI2C compatible with the one integrated on i.MX7ULP soc
- - "fsl,imx8qxp-lpi2c" for LPI2C compatible with the one integrated on i.MX8QXP soc
- - "fsl,imx8qm-lpi2c" for LPI2C compatible with the one integrated on i.MX8QM soc
-- reg : address and length of the lpi2c master registers
-- interrupts : lpi2c interrupt
-- clocks : lpi2c clock specifier
-
-Examples:
-
-lpi2c7: lpi2c7@40a50000 {
- compatible = "fsl,imx7ulp-lpi2c";
- reg = <0x40A50000 0x10000>;
- interrupt-parent = <&intc>;
- interrupts = <GIC_SPI 37 IRQ_TYPE_LEVEL_HIGH>;
- clocks = <&clks IMX7ULP_CLK_LPI2C7>;
-};
diff --git a/Documentation/devicetree/bindings/i2c/i2c-imx-lpi2c.yaml b/Documentation/devicetree/bindings/i2c/i2c-imx-lpi2c.yaml
new file mode 100644
index 0000000..3c0be0c
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/i2c-imx-lpi2c.yaml
@@ -0,0 +1,45 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/i2c/i2c-imx-lpi2c.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Freescale Low Power Inter IC (LPI2C) for i.MX
+
+maintainers:
+ - Anson Huang <Anson.Huang@nxp.com>
+
+properties:
+ compatible:
+ enum:
+ - fsl,imx7ulp-lpi2c
+ - fsl,imx8qxp-lpi2c
+ - fsl,imx8qm-lpi2c
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ clocks:
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - clocks
+
+examples:
+ - |
+ #include <dt-bindings/clock/imx7ulp-clock.h>
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+ lpi2c7@40a50000 {
+ compatible = "fsl,imx7ulp-lpi2c";
+ reg = <0x40A50000 0x10000>;
+ interrupt-parent = <&intc>;
+ interrupts = <GIC_SPI 37 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clks IMX7ULP_CLK_LPI2C7>;
+ };
--
2.7.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 2/3] dt-bindings: i2c: Convert mxs i2c to json-schema
From: Anson Huang @ 2020-06-03 1:58 UTC (permalink / raw)
To: aisheng.dong, robh+dt, shawnguo, s.hauer, kernel, festevam, linux,
pandy.gao, wolfram, linux-i2c, devicetree, linux-arm-kernel,
linux-kernel
Cc: Linux-imx
In-Reply-To: <1591149535-342-1-git-send-email-Anson.Huang@nxp.com>
Convert the MXS I2C binding to DT schema format using json-schema
Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
Documentation/devicetree/bindings/i2c/i2c-mxs.txt | 25 ----------
Documentation/devicetree/bindings/i2c/i2c-mxs.yaml | 55 ++++++++++++++++++++++
2 files changed, 55 insertions(+), 25 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/i2c/i2c-mxs.txt
create mode 100644 Documentation/devicetree/bindings/i2c/i2c-mxs.yaml
diff --git a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt b/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
deleted file mode 100644
index 4e1c8ac..0000000
--- a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
+++ /dev/null
@@ -1,25 +0,0 @@
-* Freescale MXS Inter IC (I2C) Controller
-
-Required properties:
-- compatible: Should be "fsl,<chip>-i2c"
-- reg: Should contain registers location and length
-- interrupts: Should contain ERROR interrupt number
-- clock-frequency: Desired I2C bus clock frequency in Hz.
- Only 100000Hz and 400000Hz modes are supported.
-- dmas: DMA specifier, consisting of a phandle to DMA controller node
- and I2C DMA channel ID.
- Refer to dma.txt and fsl-mxs-dma.txt for details.
-- dma-names: Must be "rx-tx".
-
-Examples:
-
-i2c0: i2c@80058000 {
- #address-cells = <1>;
- #size-cells = <0>;
- compatible = "fsl,imx28-i2c";
- reg = <0x80058000 2000>;
- interrupts = <111>;
- clock-frequency = <100000>;
- dmas = <&dma_apbx 6>;
- dma-names = "rx-tx";
-};
diff --git a/Documentation/devicetree/bindings/i2c/i2c-mxs.yaml b/Documentation/devicetree/bindings/i2c/i2c-mxs.yaml
new file mode 100644
index 0000000..7adcba3
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/i2c-mxs.yaml
@@ -0,0 +1,55 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/i2c/i2c-mxs.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Freescale MXS Inter IC (I2C) Controller
+
+maintainers:
+ - Shawn Guo <shawn.guo@linaro.org>
+
+properties:
+ compatible:
+ enum:
+ - fsl,imx23-i2c
+ - fsl,imx28-i2c
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ clock-frequency:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: |
+ Desired I2C bus clock frequency in Hz, only 100000Hz and 400000Hz
+ modes are supported.
+ default: 100000
+
+ dmas:
+ maxItems: 1
+
+ dma-names:
+ const: rx-tx
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - dmas
+ - dma-names
+
+examples:
+ - |
+ i2c@80058000 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "fsl,imx28-i2c";
+ reg = <0x80058000 2000>;
+ interrupts = <111>;
+ clock-frequency = <100000>;
+ dmas = <&dma_apbx 6>;
+ dma-names = "rx-tx";
+ };
--
2.7.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox