* Re: [PATCH 2/6] pwm: sun4i: Add a quirk for reset line
From: Maxime Ripard @ 2019-07-29 16:37 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Chen-Yu Tsai, Jernej Skrabec, Thierry Reding, Rob Herring,
Mark Rutland, linux-pwm, devicetree, linux-arm-kernel,
linux-kernel, linux-sunxi, Philipp Zabel
In-Reply-To: <20190729071218.bukw7vxilqy523k3@pengutronix.de>
[-- Attachment #1: Type: text/plain, Size: 2093 bytes --]
On Mon, Jul 29, 2019 at 09:12:18AM +0200, Uwe Kleine-König wrote:
> Hello,
>
> On Mon, Jul 29, 2019 at 02:43:23PM +0800, Chen-Yu Tsai wrote:
> > On Mon, Jul 29, 2019 at 2:36 PM Uwe Kleine-König
> > <u.kleine-koenig@pengutronix.de> wrote:
> > > On Fri, Jul 26, 2019 at 08:40:41PM +0200, Jernej Skrabec wrote:
> > > > @@ -371,6 +374,14 @@ static int sun4i_pwm_probe(struct platform_device *pdev)
> > > > if (IS_ERR(pwm->clk))
> > > > return PTR_ERR(pwm->clk);
> > > >
> > > > + if (pwm->data->has_reset) {
> > > > + pwm->rst = devm_reset_control_get(&pdev->dev, NULL);
> > > > + if (IS_ERR(pwm->rst))
> > > > + return PTR_ERR(pwm->rst);
> > > > +
> > > > + reset_control_deassert(pwm->rst);
> > > > + }
> > > > +
> > >
> > > I wonder why there is a need to track if a given chip needs a reset
> > > line. I'd just use devm_reset_control_get_optional() and drop the
> > > .has_reset member in struct sun4i_pwm_data.
> >
> > Because it's not optional for this platform, i.e. it won't work if
> > the reset control (or clk, in the next patch) is somehow missing from
> > the device tree.
>
> If the device tree is wrong it is considered ok that the driver doesn't
> behave correctly. So this is not a problem you need (or should) care
> about.
To some extent that's true, but if we can make the life easier for
everyone by reporting an error and bailing out instead of silently
ignoring that, continuing to probe and just ending up with a
completely broken system for no apparent reason, then why not just do
that?
I mean, all it takes is three lines of code.
It's no different than just calling clk_get, and testing the return
code to see if it's there or not. I wouldn't call that check when you
depend on a clock "validating the DT". It's just making sure that all
the resources needed for you to operate properly are there, which is a
pretty common thing to do.
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* [PATCH 2/2] arm64: dts: qcom: Extend AOSS QMP node
From: Thara Gopinath @ 2019-07-29 16:33 UTC (permalink / raw)
To: linux-arm-msm, agross, robh+dt, mark.rutland; +Cc: devicetree, linux-kernel
In-Reply-To: <1564418001-24940-1-git-send-email-thara.gopinath@linaro.org>
AOSS hosts resources that can be used to warm up the SoC.
Add nodes for these resources.
Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>
---
arch/arm64/boot/dts/qcom/sdm845.dtsi | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index 4babff5..d0c0d4f 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -2411,6 +2411,14 @@
#clock-cells = <0>;
#power-domain-cells = <1>;
+
+ cx_cdev: cx {
+ #cooling-cells = <2>;
+ };
+
+ ebi_cdev: ebi {
+ #cooling-cells = <2>;
+ };
};
spmi_bus: spmi@c440000 {
--
2.1.4
^ permalink raw reply related
* [PATCH 1/2] soc: qcom: Extend AOSS QMP driver to support resources that are used to wake up the SoC.
From: Thara Gopinath @ 2019-07-29 16:33 UTC (permalink / raw)
To: linux-arm-msm, agross, robh+dt, mark.rutland; +Cc: devicetree, linux-kernel
In-Reply-To: <1564418001-24940-1-git-send-email-thara.gopinath@linaro.org>
The AOSS QMP driver is extended to communicate with the additional
resources. These resources are then registered as cooling devices
with the thermal framework.
Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>
---
drivers/soc/qcom/qcom_aoss.c | 129 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 129 insertions(+)
diff --git a/drivers/soc/qcom/qcom_aoss.c b/drivers/soc/qcom/qcom_aoss.c
index 5f88519..010877e 100644
--- a/drivers/soc/qcom/qcom_aoss.c
+++ b/drivers/soc/qcom/qcom_aoss.c
@@ -10,6 +10,8 @@
#include <linux/module.h>
#include <linux/platform_device.h>
#include <linux/pm_domain.h>
+#include <linux/thermal.h>
+#include <linux/slab.h>
#define QMP_DESC_MAGIC 0x0
#define QMP_DESC_VERSION 0x4
@@ -40,6 +42,16 @@
/* 64 bytes is enough to store the requests and provides padding to 4 bytes */
#define QMP_MSG_LEN 64
+#define QMP_NUM_COOLING_RESOURCES 2
+
+static bool qmp_cdev_init_state = 1;
+
+struct qmp_cooling_device {
+ struct thermal_cooling_device *cdev;
+ struct qmp *qmp;
+ bool state;
+};
+
/**
* struct qmp - driver state for QMP implementation
* @msgram: iomem referencing the message RAM used for communication
@@ -69,6 +81,7 @@ struct qmp {
struct clk_hw qdss_clk;
struct genpd_onecell_data pd_data;
+ struct qmp_cooling_device *cooling_devs;
};
struct qmp_pd {
@@ -385,6 +398,117 @@ static void qmp_pd_remove(struct qmp *qmp)
pm_genpd_remove(data->domains[i]);
}
+static int qmp_cdev_get_max_state(struct thermal_cooling_device *cdev,
+ unsigned long *state)
+{
+ *state = qmp_cdev_init_state;
+ return 0;
+}
+
+static int qmp_cdev_get_cur_state(struct thermal_cooling_device *cdev,
+ unsigned long *state)
+{
+ struct qmp_cooling_device *qmp_cdev = cdev->devdata;
+
+ *state = qmp_cdev->state;
+ return 0;
+}
+
+static int qmp_cdev_set_cur_state(struct thermal_cooling_device *cdev,
+ unsigned long state)
+{
+ struct qmp_cooling_device *qmp_cdev = cdev->devdata;
+ char buf[QMP_MSG_LEN] = {};
+ bool cdev_state;
+ int ret;
+
+ /* Normalize state */
+ cdev_state = !!state;
+
+ if (qmp_cdev->state == state)
+ return 0;
+
+ snprintf(buf, sizeof(buf),
+ "{class: volt_flr, event:zero_temp, res:%s, value:%s}",
+ qmp_cdev->name,
+ cdev_state ? "off" : "on");
+
+ ret = qmp_send(qmp_cdev->qmp, buf, sizeof(buf));
+
+ if (!ret)
+ qmp_cdev->state = cdev_state;
+
+ return ret;
+}
+
+static struct thermal_cooling_device_ops qmp_cooling_device_ops = {
+ .get_max_state = qmp_cdev_get_max_state,
+ .get_cur_state = qmp_cdev_get_cur_state,
+ .set_cur_state = qmp_cdev_set_cur_state,
+};
+
+static int qmp_cooling_device_add(struct qmp *qmp,
+ struct qmp_cooling_device *qmp_cdev,
+ struct device_node *node)
+{
+ char *cdev_name = (char *)node->name;
+
+ qmp_cdev->qmp = qmp;
+ qmp_cdev->state = qmp_cdev_init_state;
+ qmp_cdev->cdev = devm_thermal_of_cooling_device_register
+ (qmp->dev, node,
+ cdev_name,
+ qmp_cdev, &qmp_cooling_device_ops);
+
+ if (IS_ERR(qmp_cdev->cdev))
+ dev_err(qmp->dev, "unable to register %s cooling device\n",
+ cdev_name);
+
+ return PTR_ERR_OR_ZERO(qmp_cdev->cdev);
+}
+
+static int qmp_cooling_devices_register(struct qmp *qmp)
+{
+ struct device_node *np, *child;
+ int count = QMP_NUM_COOLING_RESOURCES;
+ int ret;
+
+ np = qmp->dev->of_node;
+
+ qmp->cooling_devs = devm_kcalloc(qmp->dev, count,
+ sizeof(*qmp->cooling_devs),
+ GFP_KERNEL);
+
+ if (!qmp->cooling_devs)
+ return -ENOMEM;
+
+ for_each_available_child_of_node(np, child) {
+ if (!of_find_property(child, "#cooling-cells", NULL))
+ continue;
+ ret = qmp_cooling_device_add(qmp, &qmp->cooling_devs[count++],
+ child);
+ if (ret)
+ goto uroll_cooling_devices;
+ }
+
+ return 0;
+
+uroll_cooling_devices:
+ while (--count >= 0)
+ thermal_cooling_device_unregister
+ (qmp->cooling_devs[count].cdev);
+
+ return ret;
+}
+
+static void qmp_cooling_devices_remove(struct qmp *qmp)
+{
+ int i;
+
+ for (i = 0; i < QMP_NUM_COOLING_RESOURCES; i++)
+ thermal_cooling_device_unregister(qmp->cooling_devs[i].cdev);
+}
+
static int qmp_probe(struct platform_device *pdev)
{
struct resource *res;
@@ -433,6 +557,10 @@ static int qmp_probe(struct platform_device *pdev)
if (ret)
goto err_remove_qdss_clk;
+ ret = qmp_cooling_devices_register(qmp);
+ if (ret)
+ dev_err(&pdev->dev, "failed to register aoss cooling devices\n");
+
platform_set_drvdata(pdev, qmp);
return 0;
@@ -453,6 +581,7 @@ static int qmp_remove(struct platform_device *pdev)
qmp_qdss_clk_remove(qmp);
qmp_pd_remove(qmp);
+ qmp_cooling_devices_remove(qmp);
qmp_close(qmp);
mbox_free_channel(qmp->mbox_chan);
--
2.1.4
^ permalink raw reply related
* [PATCH 0/2] Add support for AOSS resources that are used to warm up the SoC
From: Thara Gopinath @ 2019-07-29 16:33 UTC (permalink / raw)
To: linux-arm-msm, agross, robh+dt, mark.rutland; +Cc: devicetree, linux-kernel
The Always On Sub System (AOSS) hosts certain resources
that are used to warm up the soc if the temperature falls
below certain threshold. These resources are
added can be considered as thermal warming devices
(opposite of thermal cooling devices).
These resources are controlled via AOSS QMP protocol
In kernel, these devices can be treated the same way as any other
thermal cooling device and hence are registered with the thermal
cooling framework.
To use these resources as warming devices require further tweaks in
the thermal framework which are out of scope of this patch series.
Thara Gopinath (2):
soc: qcom: Extend AOSS QMP driver to support resources that are used
to wake up the SoC.
arm64: dts: qcom: Extend AOSS QMP node
arch/arm64/boot/dts/qcom/sdm845.dtsi | 8 +++
drivers/soc/qcom/qcom_aoss.c | 129 +++++++++++++++++++++++++++++++++++
2 files changed, 137 insertions(+)
--
2.1.4
^ permalink raw reply
* Re: [PATCH 5/6] pwm: sun4i: Add support to output source clock directly
From: Uwe Kleine-König @ 2019-07-29 16:29 UTC (permalink / raw)
To: Jernej Škrabec
Cc: thierry.reding, mripard, wens, robh+dt, mark.rutland, linux-pwm,
devicetree, linux-arm-kernel, linux-kernel, linux-sunxi
In-Reply-To: <2499807.IN78SsLMYo@jernej-laptop>
Hello,
On Mon, Jul 29, 2019 at 06:16:55PM +0200, Jernej Škrabec wrote:
> Dne ponedeljek, 29. julij 2019 ob 09:06:05 CEST je Uwe Kleine-König
> napisal(a):
> > On Fri, Jul 26, 2019 at 08:40:44PM +0200, Jernej Skrabec wrote:
> > > PWM core has an option to bypass whole logic and output unchanged source
> > > clock as PWM output. This is achieved by enabling bypass bit.
> > >
> > > Note that when bypass is enabled, no other setting has any meaning, not
> > > even enable bit.
> > >
> > > This mode of operation is needed to achieve high enough frequency to
> > > serve as clock source for AC200 chip, which is integrated into same
> > > package as H6 SoC.
> > >
> > > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> > > ---
> > >
> > > drivers/pwm/pwm-sun4i.c | 31 ++++++++++++++++++++++++++++++-
> > > 1 file changed, 30 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c
> > > index 9e0eca79ff88..848cff26f385 100644
> > > --- a/drivers/pwm/pwm-sun4i.c
> > > +++ b/drivers/pwm/pwm-sun4i.c
> > > @@ -120,6 +120,19 @@ static void sun4i_pwm_get_state(struct pwm_chip
> > > *chip,
> > >
> > > val = sun4i_pwm_readl(sun4i_pwm, PWM_CTRL_REG);
> > >
> > > + /*
> > > + * PWM chapter in H6 manual has a diagram which explains that if bypass
> > > + * bit is set, no other setting has any meaning. Even more, experiment
> > > + * proved that also enable bit is ignored in this case.
> > > + */
> > > + if (val & BIT_CH(PWM_BYPASS, pwm->hwpwm)) {
> > > + state->period = DIV_ROUND_CLOSEST_ULL(NSEC_PER_SEC, clk_rate);
> > > + state->duty_cycle = state->period / 2;
> > > + state->polarity = PWM_POLARITY_NORMAL;
> > > + state->enabled = true;
> > > + return;
> > > + }
> > > +
> > >
> > > if ((PWM_REG_PRESCAL(val, pwm->hwpwm) == PWM_PRESCAL_MASK) &&
> > >
> > > sun4i_pwm->data->has_prescaler_bypass)
> > >
> > > prescaler = 1;
> > >
> > > @@ -211,7 +224,8 @@ static int sun4i_pwm_apply(struct pwm_chip *chip,
> > > struct pwm_device *pwm,>
> > > {
> > >
> > > struct sun4i_pwm_chip *sun4i_pwm = to_sun4i_pwm_chip(chip);
> > > struct pwm_state cstate;
> > >
> > > - u32 ctrl;
> > > + u32 ctrl, clk_rate;
> > > + bool bypass;
> > >
> > > int ret;
> > > unsigned int delay_us;
> > > unsigned long now;
> > >
> > > @@ -226,6 +240,16 @@ static int sun4i_pwm_apply(struct pwm_chip *chip,
> > > struct pwm_device *pwm,>
> > > }
> > >
> > > }
> > >
> > > + /*
> > > + * Although it would make much more sense to check for bypass in
> > > + * sun4i_pwm_calculate(), value of bypass bit also depends on "enabled".
> > > + * Period is allowed to be rounded up or down.
> > > + */
> >
> > Every driver seems to implement rounding the way its driver considers it
> > sensible. @Thierry: This is another patch where it would be good to have
> > a global directive about how rounding is supposed to work to provide the
> > users an reliable and uniform way to work with PWMs.
> >
> > > + clk_rate = clk_get_rate(sun4i_pwm->clk);
> > > + bypass = (state->period == NSEC_PER_SEC / clk_rate ||
> > > + state->period == DIV_ROUND_UP(NSEC_PER_SEC, clk_rate)) &&
> > > + state->enabled;
> >
> > Not sure if the compiler is clever enough to notice the obvious
> > optimisation with this code construct, but you can write this test in a
> > more clever way which has zero instead of up to two divisions. Something
> > like:
> >
> > bypass = ((state->period * clk_rate >= NSEC_PER_SEC &&
> > state->period * clk_rate < NSEC_PER_SEC + clk_rate) &&
> > state->enabled);
> >
> > In the commit log you write the motivation for using bypass is that it
> > allows to implement higher frequency then with the "normal" operation.
> > As you don't skip calculating the normal parameters requesting such a
> > high-frequency setting either errors out or doesn't catch the impossible
> > request. In both cases there is something to fix.
>
> It's the latter, otherwise it wouldn't work for my case. I'll fix the check and
> skip additional logic.
Great.
> > > +
> > >
> > > spin_lock(&sun4i_pwm->ctrl_lock);
> > > ctrl = sun4i_pwm_readl(sun4i_pwm, PWM_CTRL_REG);
> > >
> > > @@ -273,6 +297,11 @@ static int sun4i_pwm_apply(struct pwm_chip *chip,
> > > struct pwm_device *pwm,>
> > > ctrl &= ~BIT_CH(PWM_CLK_GATING, pwm->hwpwm);
> > >
> > > }
> > >
> > > + if (bypass)
> > > + ctrl |= BIT_CH(PWM_BYPASS, pwm->hwpwm);
> > > + else
> > > + ctrl &= ~BIT_CH(PWM_BYPASS, pwm->hwpwm);
> > > +
> >
> > Does switching on (or off) the bypass bit complete the currently running
> > period?
>
> I don't really know. If I understand correctly, it just bypasses PWM logic
> completely, so I would say it doesn't complete the currently running period.
This is a bug. It's part of the promise of the PWM API that started
periods are completed. Please at least document this limitation at the
top of the driver. drivers/pwm/pwm-sifive.c has an example you might
want to use as a template.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* Re: Re: [PATCH 4/6] pwm: sun4i: Add support for H6 PWM
From: Uwe Kleine-König @ 2019-07-29 16:24 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: Mark Rutland, linux-pwm-u79uwXL29TY76Z2rM5mHXA,
Jernej Škrabec, devicetree, linux-kernel, Maxime Ripard,
linux-sunxi, Rob Herring, Thierry Reding, Sascha Hauer,
linux-arm-kernel
In-Reply-To: <CAGb2v66C=ghjck6rxTg6Vt4xN2DcXntzVOa=KJWh98KRjkhnHQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Hello,
On Tue, Jul 30, 2019 at 12:09:40AM +0800, Chen-Yu Tsai wrote:
> On Tue, Jul 30, 2019 at 12:07 AM Uwe Kleine-König
> <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> wrote:
> > On Mon, Jul 29, 2019 at 05:55:52PM +0200, Jernej Škrabec wrote:
> > > Dne ponedeljek, 29. julij 2019 ob 08:40:30 CEST je Uwe Kleine-König
> > > napisal(a):
> > > > On Fri, Jul 26, 2019 at 08:40:43PM +0200, Jernej Skrabec wrote:
> > > > > --- a/drivers/pwm/pwm-sun4i.c
> > > > > +++ b/drivers/pwm/pwm-sun4i.c
> > > > > @@ -331,6 +331,13 @@ static const struct sun4i_pwm_data
> > > > > sun4i_pwm_single_bypass = {>
> > > > > .npwm = 1,
> > > > >
> > > > > };
> > > > >
> > > > > +static const struct sun4i_pwm_data sun50i_pwm_dual_bypass_clk_rst = {
> > > > > + .has_bus_clock = true,
> > > > > + .has_prescaler_bypass = true,
> > > > > + .has_reset = true,
> > > > > + .npwm = 2,
> > > > > +};
> > > > > +
> > > > >
> > > > > static const struct of_device_id sun4i_pwm_dt_ids[] = {
> > > > >
> > > > > {
> > > > >
> > > > > .compatible = "allwinner,sun4i-a10-pwm",
> > > > >
> > > > > @@ -347,6 +354,9 @@ static const struct of_device_id sun4i_pwm_dt_ids[] =
> > > > > {
> > > > >
> > > > > }, {
> > > > >
> > > > > .compatible = "allwinner,sun8i-h3-pwm",
> > > > > .data = &sun4i_pwm_single_bypass,
> > > > >
> > > > > + }, {
> > > > > + .compatible = "allwinner,sun50i-h6-pwm",
> > > > > + .data = &sun50i_pwm_dual_bypass_clk_rst,
> > > >
> > > > If you follow my suggestion for the two previous patches, you can just
> > > > use:
> > > >
> > > > compatible = "allwinner,sun50i-h6-pwm", "allwinner,sun5i-a10s-pwm";
> > > >
> > > > and drop this patch.
> > >
> > > Maxime found out that it's not compatible with A10s due to difference in bypass
> > > bit, but yes, I know what you mean.
> > >
> > > Since H6 requires reset line and bus clock to be specified, it's not compatible
> > > from DT binding side. New yaml based binding must somehow know that in order
> > > to be able to validate DT node, so it needs standalone compatible. However,
> > > depending on conclusions of other discussions, this new compatible can be
> > > associated with already available quirks structure or have it's own.
> >
> > I cannot follow. You should be able to specify in the binding that the
> > reset line and bus clock is optional. Then allwinner,sun50i-h6-pwm
> > without a reset line and bus clock also verifies, but this doesn't
> > really hurt (and who knows, maybe the next allwinner chip needs exactly
> > this).
>
> It is not optional. It will not work if either the clocks or reset controls
> are missing. How would these be optional anyway? Either it's connected and
> thus required, or it's not and therefore should be omitted from the
> description.
[Just arguing about the clock here, the argumentation is analogous for
the reset control.]
>From the driver's perspective it's optional: There are devices with and
without a bus clock. This doesn't mean that you can just ignore this
clock if it's specified. It's optional in the sense "If dt doesn't
specify it, then assume this is a device that doesn't have it and so you
don't need to handle it." but not in the sense "it doesn't matter if
you handle it or not.".
Other than that I'm on your side. So for example I think it's not
optimal that gpiod_get_optional returns NULL if GPIOLIB=n or that
devm_reset_control_get_optional returns NULL if RESET_CONTROLLER=n
because this hides exactly the kind of problem you point out here.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
To view this discussion on the web, visit https://groups.google.com/d/msgid/linux-sunxi/20190729162428.bxuzgxg5sjqptlbp%40pengutronix.de.
^ permalink raw reply
* Re: [PATCH 5/6] pwm: sun4i: Add support to output source clock directly
From: Jernej Škrabec @ 2019-07-29 16:16 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: thierry.reding-Re5JQEeQqe8AvxtiuMwx3w,
mripard-DgEjT+Ai2ygdnm+yROfE0A, wens-jdAy2FN1RRM,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
linux-pwm-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <20190729070605.vlu7kgzn362ph2q3-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Hi Uwe,
Dne ponedeljek, 29. julij 2019 ob 09:06:05 CEST je Uwe Kleine-König
napisal(a):
> On Fri, Jul 26, 2019 at 08:40:44PM +0200, Jernej Skrabec wrote:
> > PWM core has an option to bypass whole logic and output unchanged source
> > clock as PWM output. This is achieved by enabling bypass bit.
> >
> > Note that when bypass is enabled, no other setting has any meaning, not
> > even enable bit.
> >
> > This mode of operation is needed to achieve high enough frequency to
> > serve as clock source for AC200 chip, which is integrated into same
> > package as H6 SoC.
> >
> > Signed-off-by: Jernej Skrabec <jernej.skrabec-gGgVlfcn5nU@public.gmane.org>
> > ---
> >
> > drivers/pwm/pwm-sun4i.c | 31 ++++++++++++++++++++++++++++++-
> > 1 file changed, 30 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c
> > index 9e0eca79ff88..848cff26f385 100644
> > --- a/drivers/pwm/pwm-sun4i.c
> > +++ b/drivers/pwm/pwm-sun4i.c
> > @@ -120,6 +120,19 @@ static void sun4i_pwm_get_state(struct pwm_chip
> > *chip,
> >
> > val = sun4i_pwm_readl(sun4i_pwm, PWM_CTRL_REG);
> >
> > + /*
> > + * PWM chapter in H6 manual has a diagram which explains that if
bypass
> > + * bit is set, no other setting has any meaning. Even more,
experiment
> > + * proved that also enable bit is ignored in this case.
> > + */
> > + if (val & BIT_CH(PWM_BYPASS, pwm->hwpwm)) {
> > + state->period = DIV_ROUND_CLOSEST_ULL(NSEC_PER_SEC,
clk_rate);
> > + state->duty_cycle = state->period / 2;
> > + state->polarity = PWM_POLARITY_NORMAL;
> > + state->enabled = true;
> > + return;
> > + }
> > +
> >
> > if ((PWM_REG_PRESCAL(val, pwm->hwpwm) == PWM_PRESCAL_MASK) &&
> >
> > sun4i_pwm->data->has_prescaler_bypass)
> >
> > prescaler = 1;
> >
> > @@ -211,7 +224,8 @@ static int sun4i_pwm_apply(struct pwm_chip *chip,
> > struct pwm_device *pwm,>
> > {
> >
> > struct sun4i_pwm_chip *sun4i_pwm = to_sun4i_pwm_chip(chip);
> > struct pwm_state cstate;
> >
> > - u32 ctrl;
> > + u32 ctrl, clk_rate;
> > + bool bypass;
> >
> > int ret;
> > unsigned int delay_us;
> > unsigned long now;
> >
> > @@ -226,6 +240,16 @@ static int sun4i_pwm_apply(struct pwm_chip *chip,
> > struct pwm_device *pwm,>
> > }
> >
> > }
> >
> > + /*
> > + * Although it would make much more sense to check for bypass in
> > + * sun4i_pwm_calculate(), value of bypass bit also depends on
"enabled".
> > + * Period is allowed to be rounded up or down.
> > + */
>
> Every driver seems to implement rounding the way its driver considers it
> sensible. @Thierry: This is another patch where it would be good to have
> a global directive about how rounding is supposed to work to provide the
> users an reliable and uniform way to work with PWMs.
>
> > + clk_rate = clk_get_rate(sun4i_pwm->clk);
> > + bypass = (state->period == NSEC_PER_SEC / clk_rate ||
> > + state->period == DIV_ROUND_UP(NSEC_PER_SEC, clk_rate))
&&
> > + state->enabled;
>
> Not sure if the compiler is clever enough to notice the obvious
> optimisation with this code construct, but you can write this test in a
> more clever way which has zero instead of up to two divisions. Something
> like:
>
> bypass = ((state->period * clk_rate >= NSEC_PER_SEC &&
> state->period * clk_rate < NSEC_PER_SEC + clk_rate) &&
> state->enabled);
>
> In the commit log you write the motivation for using bypass is that it
> allows to implement higher frequency then with the "normal" operation.
> As you don't skip calculating the normal parameters requesting such a
> high-frequency setting either errors out or doesn't catch the impossible
> request. In both cases there is something to fix.
It's the latter, otherwise it wouldn't work for my case. I'll fix the check and
skip additional logic.
>
> > +
> >
> > spin_lock(&sun4i_pwm->ctrl_lock);
> > ctrl = sun4i_pwm_readl(sun4i_pwm, PWM_CTRL_REG);
> >
> > @@ -273,6 +297,11 @@ static int sun4i_pwm_apply(struct pwm_chip *chip,
> > struct pwm_device *pwm,>
> > ctrl &= ~BIT_CH(PWM_CLK_GATING, pwm->hwpwm);
> >
> > }
> >
> > + if (bypass)
> > + ctrl |= BIT_CH(PWM_BYPASS, pwm->hwpwm);
> > + else
> > + ctrl &= ~BIT_CH(PWM_BYPASS, pwm->hwpwm);
> > +
>
> Does switching on (or off) the bypass bit complete the currently running
> period?
I don't really know. If I understand correctly, it just bypasses PWM logic
completely, so I would say it doesn't complete the currently running period.
Take a look at chapter 3.9.2 http://linux-sunxi.org/
File:Allwinner_H6_V200_User_Manual_V1.1.pdf
Best regards,
Jernej
>
> > sun4i_pwm_writel(sun4i_pwm, ctrl, PWM_CTRL_REG);
> >
> > spin_unlock(&sun4i_pwm->ctrl_lock);
>
> Best regards
> Uwe
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
To view this discussion on the web, visit https://groups.google.com/d/msgid/linux-sunxi/2499807.IN78SsLMYo%40jernej-laptop.
^ permalink raw reply
* Re: [PATCH 3/6] pwm: sun4i: Add a quirk for bus clock
From: Uwe Kleine-König @ 2019-07-29 16:14 UTC (permalink / raw)
To: Jernej Škrabec
Cc: mark.rutland-5wv7dgnIgG8, linux-pwm-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
mripard-DgEjT+Ai2ygdnm+yROfE0A, wens-jdAy2FN1RRM,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
thierry.reding-Re5JQEeQqe8AvxtiuMwx3w,
kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <4022372.WfP88Fa4Lu@jernej-laptop>
Hello Jernej,
On Mon, Jul 29, 2019 at 05:48:36PM +0200, Jernej Škrabec wrote:
> Dne ponedeljek, 29. julij 2019 ob 08:38:25 CEST je Uwe Kleine-König
> napisal(a):
> > Hello,
> >
> > On Fri, Jul 26, 2019 at 08:40:42PM +0200, Jernej Skrabec wrote:
> > > H6 PWM core needs bus clock to be enabled in order to work.
> > >
> > > Add a quirk for it.
> > >
> > > Signed-off-by: Jernej Skrabec <jernej.skrabec-gGgVlfcn5nU@public.gmane.org>
> > > ---
> > >
> > > drivers/pwm/pwm-sun4i.c | 15 +++++++++++++++
> > > 1 file changed, 15 insertions(+)
> > >
> > > diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c
> > > index 1b7be8fbde86..7d3ac3f2dc3f 100644
> > > --- a/drivers/pwm/pwm-sun4i.c
> > > +++ b/drivers/pwm/pwm-sun4i.c
> > > @@ -72,6 +72,7 @@ static const u32 prescaler_table[] = {
> > >
> > > };
> > >
> > > struct sun4i_pwm_data {
> > >
> > > + bool has_bus_clock;
> > >
> > > bool has_prescaler_bypass;
> > > bool has_reset;
> > > unsigned int npwm;
> > >
> > > @@ -79,6 +80,7 @@ struct sun4i_pwm_data {
> > >
> > > struct sun4i_pwm_chip {
> > >
> > > struct pwm_chip chip;
> > >
> > > + struct clk *bus_clk;
> > >
> > > struct clk *clk;
> > > struct reset_control *rst;
> > > void __iomem *base;
> > >
> > > @@ -382,6 +384,16 @@ static int sun4i_pwm_probe(struct platform_device
> > > *pdev)>
> > > reset_control_deassert(pwm->rst);
> > >
> > > }
> > >
> > > + if (pwm->data->has_bus_clock) {
> > > + pwm->bus_clk = devm_clk_get(&pdev->dev, "bus");
> >
> > Similar to my suggestion in patch 2: I'd use devm_clk_get_optional() and
> > drop struct sun4i_pwm_data::has_bus_clock.
>
> This one is not so simple. This patch has incorrect logic. Correct logic would
> be to use "devm_clk_get(&pdev->dev, NULL)" for variants without bus clock as
> it is done already and "devm_clk_get(&pdev->dev, "bus")" and
> "devm_clk_get(&pdev->dev, "mod")" for variants with bus clock.
Then maybe something like the following?:
busclk = devm_clk_get_optional(..., "bus");
modclk = devm_clk_get_optional(..., "mod");
/*
* old dtbs might have a single clock but no clock names. Fall
* back to this for compatibility reasons.
*/
if (!modclk) {
modclk = devm_clk_get(..., NULL);
}
> You see, DT nodes for other variants don't have clock-names property at all.
> If it would be specified, it would be "mod". So, DT nodes for other variants
> have "mod" clock specified on first place (the only one), while H6 DT node will
> have "mod" clock specified on second place (see one of e-mails from Maxime), so
> "NULL" can't be used instead of "mod" in both cases.
>
> So I would say quirk is beneficial to know if you have to look up clocks by
> name or you just take first clock specified.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
To view this discussion on the web, visit https://groups.google.com/d/msgid/linux-sunxi/20190729161435.5bnj3ikocsyep4dd%40pengutronix.de.
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/6] pwm: sun4i: Add support for H6 PWM
From: Chen-Yu Tsai @ 2019-07-29 16:09 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Jernej Škrabec, Mark Rutland, linux-pwm, devicetree,
linux-sunxi, linux-kernel, Maxime Ripard, Rob Herring,
Thierry Reding, Sascha Hauer, linux-arm-kernel
In-Reply-To: <20190729160723.am3cs5pasi22pibi@pengutronix.de>
On Tue, Jul 30, 2019 at 12:07 AM Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
>
> On Mon, Jul 29, 2019 at 05:55:52PM +0200, Jernej Škrabec wrote:
> > Hi Uwe,
> >
> > Dne ponedeljek, 29. julij 2019 ob 08:40:30 CEST je Uwe Kleine-König
> > napisal(a):
> > > On Fri, Jul 26, 2019 at 08:40:43PM +0200, Jernej Skrabec wrote:
> > > > Now that sun4i PWM driver supports deasserting reset line and enabling
> > > > bus clock, support for H6 PWM can be added.
> > > >
> > > > Note that while H6 PWM has two channels, only first one is wired to
> > > > output pin. Second channel is used as a clock source to companion AC200
> > > > chip which is bundled into same package.
> > > >
> > > > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> > > > ---
> > > >
> > > > drivers/pwm/pwm-sun4i.c | 10 ++++++++++
> > > > 1 file changed, 10 insertions(+)
> > > >
> > > > diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c
> > > > index 7d3ac3f2dc3f..9e0eca79ff88 100644
> > > > --- a/drivers/pwm/pwm-sun4i.c
> > > > +++ b/drivers/pwm/pwm-sun4i.c
> > > > @@ -331,6 +331,13 @@ static const struct sun4i_pwm_data
> > > > sun4i_pwm_single_bypass = {>
> > > > .npwm = 1,
> > > >
> > > > };
> > > >
> > > > +static const struct sun4i_pwm_data sun50i_pwm_dual_bypass_clk_rst = {
> > > > + .has_bus_clock = true,
> > > > + .has_prescaler_bypass = true,
> > > > + .has_reset = true,
> > > > + .npwm = 2,
> > > > +};
> > > > +
> > > >
> > > > static const struct of_device_id sun4i_pwm_dt_ids[] = {
> > > >
> > > > {
> > > >
> > > > .compatible = "allwinner,sun4i-a10-pwm",
> > > >
> > > > @@ -347,6 +354,9 @@ static const struct of_device_id sun4i_pwm_dt_ids[] =
> > > > {
> > > >
> > > > }, {
> > > >
> > > > .compatible = "allwinner,sun8i-h3-pwm",
> > > > .data = &sun4i_pwm_single_bypass,
> > > >
> > > > + }, {
> > > > + .compatible = "allwinner,sun50i-h6-pwm",
> > > > + .data = &sun50i_pwm_dual_bypass_clk_rst,
> > >
> > > If you follow my suggestion for the two previous patches, you can just
> > > use:
> > >
> > > compatible = "allwinner,sun50i-h6-pwm", "allwinner,sun5i-a10s-pwm";
> > >
> > > and drop this patch.
> >
> > Maxime found out that it's not compatible with A10s due to difference in bypass
> > bit, but yes, I know what you mean.
> >
> > Since H6 requires reset line and bus clock to be specified, it's not compatible
> > from DT binding side. New yaml based binding must somehow know that in order
> > to be able to validate DT node, so it needs standalone compatible. However,
> > depending on conclusions of other discussions, this new compatible can be
> > associated with already available quirks structure or have it's own.
>
> I cannot follow. You should be able to specify in the binding that the
> reset line and bus clock is optional. Then allwinner,sun50i-h6-pwm
> without a reset line and bus clock also verifies, but this doesn't
> really hurt (and who knows, maybe the next allwinner chip needs exactly
> this).
It is not optional. It will not work if either the clocks or reset controls
are missing. How would these be optional anyway? Either it's connected and
thus required, or it's not and therefore should be omitted from the
description.
ChenYu
^ permalink raw reply
* Re: [PATCH 4/6] pwm: sun4i: Add support for H6 PWM
From: Uwe Kleine-König @ 2019-07-29 16:07 UTC (permalink / raw)
To: Jernej Škrabec
Cc: mark.rutland-5wv7dgnIgG8, linux-pwm-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
mripard-DgEjT+Ai2ygdnm+yROfE0A, wens-jdAy2FN1RRM,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
thierry.reding-Re5JQEeQqe8AvxtiuMwx3w,
kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <223488703.0I5IR7NXoI@jernej-laptop>
On Mon, Jul 29, 2019 at 05:55:52PM +0200, Jernej Škrabec wrote:
> Hi Uwe,
>
> Dne ponedeljek, 29. julij 2019 ob 08:40:30 CEST je Uwe Kleine-König
> napisal(a):
> > On Fri, Jul 26, 2019 at 08:40:43PM +0200, Jernej Skrabec wrote:
> > > Now that sun4i PWM driver supports deasserting reset line and enabling
> > > bus clock, support for H6 PWM can be added.
> > >
> > > Note that while H6 PWM has two channels, only first one is wired to
> > > output pin. Second channel is used as a clock source to companion AC200
> > > chip which is bundled into same package.
> > >
> > > Signed-off-by: Jernej Skrabec <jernej.skrabec-gGgVlfcn5nU@public.gmane.org>
> > > ---
> > >
> > > drivers/pwm/pwm-sun4i.c | 10 ++++++++++
> > > 1 file changed, 10 insertions(+)
> > >
> > > diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c
> > > index 7d3ac3f2dc3f..9e0eca79ff88 100644
> > > --- a/drivers/pwm/pwm-sun4i.c
> > > +++ b/drivers/pwm/pwm-sun4i.c
> > > @@ -331,6 +331,13 @@ static const struct sun4i_pwm_data
> > > sun4i_pwm_single_bypass = {>
> > > .npwm = 1,
> > >
> > > };
> > >
> > > +static const struct sun4i_pwm_data sun50i_pwm_dual_bypass_clk_rst = {
> > > + .has_bus_clock = true,
> > > + .has_prescaler_bypass = true,
> > > + .has_reset = true,
> > > + .npwm = 2,
> > > +};
> > > +
> > >
> > > static const struct of_device_id sun4i_pwm_dt_ids[] = {
> > >
> > > {
> > >
> > > .compatible = "allwinner,sun4i-a10-pwm",
> > >
> > > @@ -347,6 +354,9 @@ static const struct of_device_id sun4i_pwm_dt_ids[] =
> > > {
> > >
> > > }, {
> > >
> > > .compatible = "allwinner,sun8i-h3-pwm",
> > > .data = &sun4i_pwm_single_bypass,
> > >
> > > + }, {
> > > + .compatible = "allwinner,sun50i-h6-pwm",
> > > + .data = &sun50i_pwm_dual_bypass_clk_rst,
> >
> > If you follow my suggestion for the two previous patches, you can just
> > use:
> >
> > compatible = "allwinner,sun50i-h6-pwm", "allwinner,sun5i-a10s-pwm";
> >
> > and drop this patch.
>
> Maxime found out that it's not compatible with A10s due to difference in bypass
> bit, but yes, I know what you mean.
>
> Since H6 requires reset line and bus clock to be specified, it's not compatible
> from DT binding side. New yaml based binding must somehow know that in order
> to be able to validate DT node, so it needs standalone compatible. However,
> depending on conclusions of other discussions, this new compatible can be
> associated with already available quirks structure or have it's own.
I cannot follow. You should be able to specify in the binding that the
reset line and bus clock is optional. Then allwinner,sun50i-h6-pwm
without a reset line and bus clock also verifies, but this doesn't
really hurt (and who knows, maybe the next allwinner chip needs exactly
this).
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
To view this discussion on the web, visit https://groups.google.com/d/msgid/linux-sunxi/20190729160723.am3cs5pasi22pibi%40pengutronix.de.
^ permalink raw reply
* Re: [PATCH 4/6] pwm: sun4i: Add support for H6 PWM
From: Jernej Škrabec @ 2019-07-29 15:55 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: thierry.reding-Re5JQEeQqe8AvxtiuMwx3w,
mripard-DgEjT+Ai2ygdnm+yROfE0A, wens-jdAy2FN1RRM,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
linux-pwm-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, kernel-bIcnvbaLZ9MEGnE8C9+IrQ
In-Reply-To: <20190729064030.7uenld2kbof45zti-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Hi Uwe,
Dne ponedeljek, 29. julij 2019 ob 08:40:30 CEST je Uwe Kleine-König
napisal(a):
> On Fri, Jul 26, 2019 at 08:40:43PM +0200, Jernej Skrabec wrote:
> > Now that sun4i PWM driver supports deasserting reset line and enabling
> > bus clock, support for H6 PWM can be added.
> >
> > Note that while H6 PWM has two channels, only first one is wired to
> > output pin. Second channel is used as a clock source to companion AC200
> > chip which is bundled into same package.
> >
> > Signed-off-by: Jernej Skrabec <jernej.skrabec-gGgVlfcn5nU@public.gmane.org>
> > ---
> >
> > drivers/pwm/pwm-sun4i.c | 10 ++++++++++
> > 1 file changed, 10 insertions(+)
> >
> > diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c
> > index 7d3ac3f2dc3f..9e0eca79ff88 100644
> > --- a/drivers/pwm/pwm-sun4i.c
> > +++ b/drivers/pwm/pwm-sun4i.c
> > @@ -331,6 +331,13 @@ static const struct sun4i_pwm_data
> > sun4i_pwm_single_bypass = {>
> > .npwm = 1,
> >
> > };
> >
> > +static const struct sun4i_pwm_data sun50i_pwm_dual_bypass_clk_rst = {
> > + .has_bus_clock = true,
> > + .has_prescaler_bypass = true,
> > + .has_reset = true,
> > + .npwm = 2,
> > +};
> > +
> >
> > static const struct of_device_id sun4i_pwm_dt_ids[] = {
> >
> > {
> >
> > .compatible = "allwinner,sun4i-a10-pwm",
> >
> > @@ -347,6 +354,9 @@ static const struct of_device_id sun4i_pwm_dt_ids[] =
> > {
> >
> > }, {
> >
> > .compatible = "allwinner,sun8i-h3-pwm",
> > .data = &sun4i_pwm_single_bypass,
> >
> > + }, {
> > + .compatible = "allwinner,sun50i-h6-pwm",
> > + .data = &sun50i_pwm_dual_bypass_clk_rst,
>
> If you follow my suggestion for the two previous patches, you can just
> use:
>
> compatible = "allwinner,sun50i-h6-pwm", "allwinner,sun5i-a10s-pwm";
>
> and drop this patch.
Maxime found out that it's not compatible with A10s due to difference in bypass
bit, but yes, I know what you mean.
Since H6 requires reset line and bus clock to be specified, it's not compatible
from DT binding side. New yaml based binding must somehow know that in order
to be able to validate DT node, so it needs standalone compatible. However,
depending on conclusions of other discussions, this new compatible can be
associated with already available quirks structure or have it's own.
Best regards,
Jernej
>
> Best regards
> Uwe
>
> > }, {
> >
> > /* sentinel */
> >
> > },
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
To view this discussion on the web, visit https://groups.google.com/d/msgid/linux-sunxi/223488703.0I5IR7NXoI%40jernej-laptop.
^ permalink raw reply
* Re: [PATCH 3/6] pwm: sun4i: Add a quirk for bus clock
From: Jernej Škrabec @ 2019-07-29 15:48 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: thierry.reding-Re5JQEeQqe8AvxtiuMwx3w,
mripard-DgEjT+Ai2ygdnm+yROfE0A, wens-jdAy2FN1RRM,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
linux-pwm-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, kernel-bIcnvbaLZ9MEGnE8C9+IrQ
In-Reply-To: <20190729063825.wxfky6nswcru26g7-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Hi Uwe,
Dne ponedeljek, 29. julij 2019 ob 08:38:25 CEST je Uwe Kleine-König
napisal(a):
> Hello,
>
> On Fri, Jul 26, 2019 at 08:40:42PM +0200, Jernej Skrabec wrote:
> > H6 PWM core needs bus clock to be enabled in order to work.
> >
> > Add a quirk for it.
> >
> > Signed-off-by: Jernej Skrabec <jernej.skrabec-gGgVlfcn5nU@public.gmane.org>
> > ---
> >
> > drivers/pwm/pwm-sun4i.c | 15 +++++++++++++++
> > 1 file changed, 15 insertions(+)
> >
> > diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c
> > index 1b7be8fbde86..7d3ac3f2dc3f 100644
> > --- a/drivers/pwm/pwm-sun4i.c
> > +++ b/drivers/pwm/pwm-sun4i.c
> > @@ -72,6 +72,7 @@ static const u32 prescaler_table[] = {
> >
> > };
> >
> > struct sun4i_pwm_data {
> >
> > + bool has_bus_clock;
> >
> > bool has_prescaler_bypass;
> > bool has_reset;
> > unsigned int npwm;
> >
> > @@ -79,6 +80,7 @@ struct sun4i_pwm_data {
> >
> > struct sun4i_pwm_chip {
> >
> > struct pwm_chip chip;
> >
> > + struct clk *bus_clk;
> >
> > struct clk *clk;
> > struct reset_control *rst;
> > void __iomem *base;
> >
> > @@ -382,6 +384,16 @@ static int sun4i_pwm_probe(struct platform_device
> > *pdev)>
> > reset_control_deassert(pwm->rst);
> >
> > }
> >
> > + if (pwm->data->has_bus_clock) {
> > + pwm->bus_clk = devm_clk_get(&pdev->dev, "bus");
>
> Similar to my suggestion in patch 2: I'd use devm_clk_get_optional() and
> drop struct sun4i_pwm_data::has_bus_clock.
This one is not so simple. This patch has incorrect logic. Correct logic would
be to use "devm_clk_get(&pdev->dev, NULL)" for variants without bus clock as
it is done already and "devm_clk_get(&pdev->dev, "bus")" and
"devm_clk_get(&pdev->dev, "mod")" for variants with bus clock.
You see, DT nodes for other variants don't have clock-names property at all.
If it would be specified, it would be "mod". So, DT nodes for other variants
have "mod" clock specified on first place (the only one), while H6 DT node will
have "mod" clock specified on second place (see one of e-mails from Maxime), so
"NULL" can't be used instead of "mod" in both cases.
So I would say quirk is beneficial to know if you have to look up clocks by
name or you just take first clock specified.
Best regards,
Jernej
>
> Best regards
> Uwe
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
To view this discussion on the web, visit https://groups.google.com/d/msgid/linux-sunxi/4022372.WfP88Fa4Lu%40jernej-laptop.
^ permalink raw reply
* RE: [PATCH net-next v3 2/4] enetc: Add mdio bus driver for the PCIe MDIO endpoint
From: Claudiu Manoil @ 2019-07-29 15:46 UTC (permalink / raw)
To: Claudiu Manoil, Andrew Lunn
Cc: David S . Miller, Rob Herring, Leo Li, Alexandru Marginean,
netdev@vger.kernel.org, devicetree@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
In-Reply-To: <VI1PR04MB48806AF2F6CEDE105B78086696DD0@VI1PR04MB4880.eurprd04.prod.outlook.com>
>-----Original Message-----
>From: netdev-owner@vger.kernel.org <netdev-owner@vger.kernel.org> On
>Behalf Of Claudiu Manoil
>Sent: Monday, July 29, 2019 6:40 PM
>To: Andrew Lunn <andrew@lunn.ch>
>Cc: David S . Miller <davem@davemloft.net>; Rob Herring
><robh+dt@kernel.org>; Leo Li <leoyang.li@nxp.com>; Alexandru Marginean
><alexandru.marginean@nxp.com>; netdev@vger.kernel.org;
>devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
>kernel@vger.kernel.org
>Subject: RE: [PATCH net-next v3 2/4] enetc: Add mdio bus driver for the PCIe
>MDIO endpoint
>
>>-----Original Message-----
>>From: Andrew Lunn <andrew@lunn.ch>
>>Sent: Monday, July 29, 2019 6:35 PM
>>To: Claudiu Manoil <claudiu.manoil@nxp.com>
>>Cc: David S . Miller <davem@davemloft.net>; Rob Herring
>><robh+dt@kernel.org>; Leo Li <leoyang.li@nxp.com>; Alexandru Marginean
>><alexandru.marginean@nxp.com>; netdev@vger.kernel.org;
>>devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
>>linux- kernel@vger.kernel.org
>>Subject: Re: [PATCH net-next v3 2/4] enetc: Add mdio bus driver for the
>>PCIe MDIO endpoint
>>
>>> + hw->port = pci_iomap(pdev, 0, 0);
>>> + if (!bus->priv) {
>>
>>hw->port ??
>>
>
>Yeah, better ignore this for now 😊
>It's for the enetc accessors, enetc_port_..().
Oh I see, it's a mistake. I'm checking the wrong thing.
Sorry. Thanks for the review.
^ permalink raw reply
* RE: [PATCH net-next v3 2/4] enetc: Add mdio bus driver for the PCIe MDIO endpoint
From: Claudiu Manoil @ 2019-07-29 15:39 UTC (permalink / raw)
To: Andrew Lunn
Cc: David S . Miller, Rob Herring, Leo Li, Alexandru Marginean,
netdev@vger.kernel.org, devicetree@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20190729153524.GG4110@lunn.ch>
>-----Original Message-----
>From: Andrew Lunn <andrew@lunn.ch>
>Sent: Monday, July 29, 2019 6:35 PM
>To: Claudiu Manoil <claudiu.manoil@nxp.com>
>Cc: David S . Miller <davem@davemloft.net>; Rob Herring
><robh+dt@kernel.org>; Leo Li <leoyang.li@nxp.com>; Alexandru Marginean
><alexandru.marginean@nxp.com>; netdev@vger.kernel.org;
>devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
>kernel@vger.kernel.org
>Subject: Re: [PATCH net-next v3 2/4] enetc: Add mdio bus driver for the PCIe
>MDIO endpoint
>
>> + hw->port = pci_iomap(pdev, 0, 0);
>> + if (!bus->priv) {
>
>hw->port ??
>
Yeah, better ignore this for now 😊
It's for the enetc accessors, enetc_port_..().
^ permalink raw reply
* Re: [PATCH 0/6] Add support for Qualcomm SM8150 and SC7180 SoCs
From: Bjorn Andersson @ 2019-07-29 15:38 UTC (permalink / raw)
To: Sibi Sankar
Cc: robh+dt, vkoul, aneela, mark.rutland, agross, linux-kernel,
linux-arm-msm, devicetree, jassisinghbrar, clew
In-Reply-To: <20190729120633.20451-1-sibis@codeaurora.org>
On Mon 29 Jul 05:06 PDT 2019, Sibi Sankar wrote:
> This patch series adds SCM, APSS shared mailbox and QMP AOSS PD/clock
> support on SM8150 and SC7180 SoCs.
>
Thanks Sibi, this looks good.
Could you please update the last 5 patches to ensure/maintain sort order
of the lists they affect.
Regards,
Bjorn
> Sibi Sankar (6):
> soc: qcom: smem: Update max processor count
> dt-bindings: firmware: scm: Add SM8150 and SC7180 support
> dt-bindings: mailbox: Add APSS shared for SM8150 and SC7180 SoCs
> mailbox: qcom: Add support for Qualcomm SM8150 and SC7180 SoCs
> dt-bindings: soc: qcom: aoss: Add SM8150 and SC7180 support
> soc: qcom: aoss: Add AOSS QMP support
>
> Documentation/devicetree/bindings/firmware/qcom,scm.txt | 2 ++
> .../devicetree/bindings/mailbox/qcom,apcs-kpss-global.txt | 2 ++
> Documentation/devicetree/bindings/soc/qcom/qcom,aoss-qmp.txt | 5 ++++-
> drivers/mailbox/qcom-apcs-ipc-mailbox.c | 2 ++
> drivers/soc/qcom/qcom_aoss.c | 2 ++
> drivers/soc/qcom/smem.c | 2 +-
> 6 files changed, 13 insertions(+), 2 deletions(-)
>
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
^ permalink raw reply
* Re: [PATCH net-next v3 4/4] arm64: dts: fsl: ls1028a: Enable eth port1 on the ls1028a QDS board
From: Andrew Lunn @ 2019-07-29 15:37 UTC (permalink / raw)
To: Claudiu Manoil
Cc: devicetree, netdev, alexandru.marginean, linux-kernel, Li Yang,
Rob Herring, David S . Miller, linux-arm-kernel
In-Reply-To: <1564394627-3810-5-git-send-email-claudiu.manoil@nxp.com>
On Mon, Jul 29, 2019 at 01:03:47PM +0300, Claudiu Manoil wrote:
> LS1028a has one Ethernet management interface. On the QDS board, the
> MDIO signals are multiplexed to either on-board AR8035 PHY device or
> to 4 PCIe slots allowing for SGMII cards.
> To enable the Ethernet ENETC Port 1, which can only be connected to a
> RGMII PHY, the multiplexer needs to be configured to route the MDIO to
> the AR8035 PHY. The MDIO/MDC routing is controlled by bits 7:4 of FPGA
> board config register 0x54, and value 0 selects the on-board RGMII PHY.
> The FPGA board config registers are accessible on the i2c bus, at address
> 0x66.
>
> The PF3 MDIO PCIe integrated endpoint device allows for centralized access
> to the MDIO bus. Add the corresponding devicetree node and set it to be
> the MDIO bus parent.
>
> Signed-off-by: Alex Marginean <alexandru.marginean@nxp.com>
> Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH net-next v3 3/4] dt-bindings: net: fsl: enetc: Add bindings for the central MDIO PCIe endpoint
From: Andrew Lunn @ 2019-07-29 15:36 UTC (permalink / raw)
To: Claudiu Manoil
Cc: devicetree, netdev, alexandru.marginean, linux-kernel, Li Yang,
Rob Herring, David S . Miller, linux-arm-kernel
In-Reply-To: <1564394627-3810-4-git-send-email-claudiu.manoil@nxp.com>
On Mon, Jul 29, 2019 at 01:03:46PM +0300, Claudiu Manoil wrote:
> The on-chip PCIe root complex that integrates the ENETC ethernet
> controllers also integrates a PCIe endpoint for the MDIO controller
> providing for centralized control of the ENETC mdio bus.
> Add bindings for this "central" MDIO Integrated PCIe Endpoint.
>
> Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH net-next v3 2/4] enetc: Add mdio bus driver for the PCIe MDIO endpoint
From: Andrew Lunn @ 2019-07-29 15:35 UTC (permalink / raw)
To: Claudiu Manoil
Cc: devicetree, netdev, alexandru.marginean, linux-kernel, Li Yang,
Rob Herring, David S . Miller, linux-arm-kernel
In-Reply-To: <1564394627-3810-3-git-send-email-claudiu.manoil@nxp.com>
> + hw->port = pci_iomap(pdev, 0, 0);
> + if (!bus->priv) {
hw->port ??
Andrew
^ permalink raw reply
* Re: [v2,2/2] PCI: mediatek: Add controller support for MT7629
From: Lorenzo Pieralisi @ 2019-07-29 15:33 UTC (permalink / raw)
To: Jianjun Wang
Cc: Mark Rutland, devicetree, Ryder Lee, linux-pci, youlin.pei,
linux-kernel, Matthias Brugger, Rob Herring, linux-mediatek,
Bjorn Helgaas, linux-arm-kernel
In-Reply-To: <1564385918.17211.6.camel@mhfsdcap03>
On Mon, Jul 29, 2019 at 03:38:38PM +0800, Jianjun Wang wrote:
> On Fri, 2019-06-28 at 15:34 +0800, Jianjun Wang wrote:
> > MT7629 is an ARM platform SoC which has the same PCIe IP with MT7622.
> >
> > The HW default value of its Device ID is invalid, fix its Device ID to
> > match the hardware implementation.
> >
> > Acked-by: Ryder Lee <ryder.lee@mediatek.com>
> > Signed-off-by: Jianjun Wang <jianjun.wang@mediatek.com>
> > ---
> > drivers/pci/controller/pcie-mediatek.c | 18 ++++++++++++++++++
> > include/linux/pci_ids.h | 1 +
> > 2 files changed, 19 insertions(+)
> >
> > diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
> > index 80601e1b939e..e5e6740b635d 100644
> > --- a/drivers/pci/controller/pcie-mediatek.c
> > +++ b/drivers/pci/controller/pcie-mediatek.c
> > @@ -73,6 +73,7 @@
> > #define PCIE_MSI_VECTOR 0x0c0
> >
> > #define PCIE_CONF_VEND_ID 0x100
> > +#define PCIE_CONF_DEVICE_ID 0x102
> > #define PCIE_CONF_CLASS_ID 0x106
> >
> > #define PCIE_INT_MASK 0x420
> > @@ -141,12 +142,16 @@ struct mtk_pcie_port;
> > /**
> > * struct mtk_pcie_soc - differentiate between host generations
> > * @need_fix_class_id: whether this host's class ID needed to be fixed or not
> > + * @need_fix_device_id: whether this host's Device ID needed to be fixed or not
> > + * @device_id: Device ID which this host need to be fixed
> > * @ops: pointer to configuration access functions
> > * @startup: pointer to controller setting functions
> > * @setup_irq: pointer to initialize IRQ functions
> > */
> > struct mtk_pcie_soc {
> > bool need_fix_class_id;
> > + bool need_fix_device_id;
> > + unsigned int device_id;
> > struct pci_ops *ops;
> > int (*startup)(struct mtk_pcie_port *port);
> > int (*setup_irq)(struct mtk_pcie_port *port, struct device_node *node);
> > @@ -696,6 +701,9 @@ static int mtk_pcie_startup_port_v2(struct mtk_pcie_port *port)
> > writew(val, port->base + PCIE_CONF_CLASS_ID);
> > }
> >
> > + if (soc->need_fix_device_id)
> > + writew(soc->device_id, port->base + PCIE_CONF_DEVICE_ID);
> > +
> > /* 100ms timeout value should be enough for Gen1/2 training */
> > err = readl_poll_timeout(port->base + PCIE_LINK_STATUS_V2, val,
> > !!(val & PCIE_PORT_LINKUP_V2), 20,
> > @@ -1216,11 +1224,21 @@ static const struct mtk_pcie_soc mtk_pcie_soc_mt7622 = {
> > .setup_irq = mtk_pcie_setup_irq,
> > };
> >
> > +static const struct mtk_pcie_soc mtk_pcie_soc_mt7629 = {
> > + .need_fix_class_id = true,
> > + .need_fix_device_id = true,
> > + .device_id = PCI_DEVICE_ID_MEDIATEK_7629,
> > + .ops = &mtk_pcie_ops_v2,
> > + .startup = mtk_pcie_startup_port_v2,
> > + .setup_irq = mtk_pcie_setup_irq,
> > +};
> > +
> > static const struct of_device_id mtk_pcie_ids[] = {
> > { .compatible = "mediatek,mt2701-pcie", .data = &mtk_pcie_soc_v1 },
> > { .compatible = "mediatek,mt7623-pcie", .data = &mtk_pcie_soc_v1 },
> > { .compatible = "mediatek,mt2712-pcie", .data = &mtk_pcie_soc_mt2712 },
> > { .compatible = "mediatek,mt7622-pcie", .data = &mtk_pcie_soc_mt7622 },
> > + { .compatible = "mediatek,mt7629-pcie", .data = &mtk_pcie_soc_mt7629 },
> > {},
> > };
> >
> > diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
> > index 70e86148cb1e..aa32962759b2 100644
> > --- a/include/linux/pci_ids.h
> > +++ b/include/linux/pci_ids.h
> > @@ -2131,6 +2131,7 @@
> > #define PCI_VENDOR_ID_MYRICOM 0x14c1
> >
> > #define PCI_VENDOR_ID_MEDIATEK 0x14c3
> > +#define PCI_DEVICE_ID_MEDIATEK_7629 0x7629
> >
> > #define PCI_VENDOR_ID_TITAN 0x14D2
> > #define PCI_DEVICE_ID_TITAN_010L 0x8001
>
> Hi Bjorn & Lorenzo,
>
> Is this patch ok or is there anything I need to fixed?
We are getting to it shortly, thanks for your patience.
Lorenzo
^ permalink raw reply
* Re: [PATCH net-next v3 1/4] enetc: Clean up local mdio bus allocation
From: Andrew Lunn @ 2019-07-29 15:32 UTC (permalink / raw)
To: Claudiu Manoil
Cc: devicetree, netdev, alexandru.marginean, linux-kernel, Li Yang,
Rob Herring, David S . Miller, linux-arm-kernel
In-Reply-To: <1564394627-3810-2-git-send-email-claudiu.manoil@nxp.com>
On Mon, Jul 29, 2019 at 01:03:44PM +0300, Claudiu Manoil wrote:
> What's needed is basically a pointer to the mdio registers.
> This is one way to store it inside bus->priv allocated space,
> without upsetting sparse.
> Reworked accessors to avoid __iomem casting.
> Used devm_* variant to further clean up the init error /
> remove paths.
>
> Fixes following sparse warning:
> warning: incorrect type in assignment (different address spaces)
> expected void *priv
> got struct enetc_mdio_regs [noderef] <asn:2>*[assigned] regs
>
> Fixes: ebfcb23d62ab ("enetc: Add ENETC PF level external MDIO support")
>
> Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com>
Thanks, much nicer.
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* [PATCH] ARM: dts: stm32: add phy-dsi-supply property on stm32mp157c-ev1
From: Yannick Fertré @ 2019-07-29 14:29 UTC (permalink / raw)
To: Maxime Coquelin, Alexandre Torgue, Rob Herring, Mark Rutland,
linux-stm32, linux-arm-kernel, devicetree, linux-kernel,
Benjamin Gaignard, Yannick Fertre, Philippe Cornu,
Fabrice Gasnier
The dsi physical layer is powered by the 1v8 power controller supply.
Signed-off-by: Yannick Fertré <yannick.fertre@st.com>
---
arch/arm/boot/dts/stm32mp157c-ev1.dts | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/boot/dts/stm32mp157c-ev1.dts b/arch/arm/boot/dts/stm32mp157c-ev1.dts
index feb8f77..19d69d0 100644
--- a/arch/arm/boot/dts/stm32mp157c-ev1.dts
+++ b/arch/arm/boot/dts/stm32mp157c-ev1.dts
@@ -101,6 +101,7 @@
&dsi {
#address-cells = <1>;
#size-cells = <0>;
+ phy-dsi-supply = <®18>;
status = "okay";
ports {
--
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
* Re: [PATCH v8 08/11] usb: roles: get usb-role-switch from parent
From: Heikki Krogerus @ 2019-07-29 14:25 UTC (permalink / raw)
To: Chunfeng Yun
Cc: Rob Herring, Greg Kroah-Hartman, Biju Das, Mark Rutland,
Matthias Brugger, Adam Thomson, Li Jun, Badhri Jagan Sridharan,
Hans de Goede, Andy Shevchenko, Min Guo, devicetree, linux-kernel,
linux-usb, linux-arm-kernel, linux-mediatek, Linus Walleij
In-Reply-To: <1563958245-6321-9-git-send-email-chunfeng.yun@mediatek.com>
On Wed, Jul 24, 2019 at 04:50:42PM +0800, Chunfeng Yun wrote:
> when the USB host controller is the parent of the connector,
> usually type-B, sometimes don't need the graph, so we should
> check whether it's parent registers usb-role-switch or not
> firstly, and get it if exists.
>
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
I don't think I actually wrote the patch. I may have proposed the code
for you, but I never prepared a patch out out that. Please drop the
above Signed-off-by line if that is the case. I case I really did
write the patch, then you are missing the "From: Heikki..." first
line, but I really don't remember preparing the patch.
If the idea came from me, you can use for example the suggested-by
tag: "Suggested-by: Heikki Krogerus <heikki.krogerus...".
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> ---
> v8: no changes
> v7:
> add signed-off-by Chunfeng
>
> v6:
> new patch
> ---
> drivers/usb/roles/class.c | 25 +++++++++++++++++++++----
> 1 file changed, 21 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/roles/class.c b/drivers/usb/roles/class.c
> index 5b637aaf311f..87439a84c983 100644
> --- a/drivers/usb/roles/class.c
> +++ b/drivers/usb/roles/class.c
> @@ -114,6 +114,19 @@ static void *usb_role_switch_match(struct device_connection *con, int ep,
> return dev ? to_role_switch(dev) : ERR_PTR(-EPROBE_DEFER);
> }
>
> +static struct usb_role_switch *
> +usb_role_switch_is_parent(struct fwnode_handle *fwnode)
> +{
> + struct fwnode_handle *parent = fwnode_get_parent(fwnode);
> + struct device *dev;
> +
> + if (!parent || !fwnode_property_present(parent, "usb-role-switch"))
> + return NULL;
> +
> + dev = class_find_device(role_class, NULL, parent, switch_fwnode_match);
> + return dev ? to_role_switch(dev) : ERR_PTR(-EPROBE_DEFER);
> +}
> +
> /**
> * usb_role_switch_get - Find USB role switch linked with the caller
> * @dev: The caller device
> @@ -125,8 +138,10 @@ struct usb_role_switch *usb_role_switch_get(struct device *dev)
> {
> struct usb_role_switch *sw;
>
> - sw = device_connection_find_match(dev, "usb-role-switch", NULL,
> - usb_role_switch_match);
> + sw = usb_role_switch_is_parent(dev_fwnode(dev));
> + if (!sw)
> + sw = device_connection_find_match(dev, "usb-role-switch", NULL,
> + usb_role_switch_match);
>
> if (!IS_ERR_OR_NULL(sw))
> WARN_ON(!try_module_get(sw->dev.parent->driver->owner));
> @@ -146,8 +161,10 @@ struct usb_role_switch *fwnode_usb_role_switch_get(struct fwnode_handle *fwnode)
> {
> struct usb_role_switch *sw;
>
> - sw = fwnode_connection_find_match(fwnode, "usb-role-switch", NULL,
> - usb_role_switch_match);
> + sw = usb_role_switch_is_parent(fwnode);
> + if (!sw)
> + sw = fwnode_connection_find_match(fwnode, "usb-role-switch",
> + NULL, usb_role_switch_match);
> if (!IS_ERR_OR_NULL(sw))
> WARN_ON(!try_module_get(sw->dev.parent->driver->owner));
>
> --
> 2.21.0
thanks,
--
heikki
^ permalink raw reply
* Re: [PATCH v3 6a/7] dt-bindings: Add ANX6345 DP/eDP transmitter binding
From: Torsten Duwe @ 2019-07-29 14:22 UTC (permalink / raw)
To: Rob Herring, Maxime Ripard
Cc: Chen-Yu Tsai, Mark Rutland, Thierry Reding, David Airlie,
Daniel Vetter, Andrzej Hajda, Laurent Pinchart, Icenowy Zheng,
Sean Paul, Vasily Khoruzhick, Harald Geyer, Greg Kroah-Hartman,
Thomas Gleixner, dri-devel, devicetree, linux-arm-kernel,
linux-kernel
In-Reply-To: <20190726163601.o32bxqew5xavjgyi@flea>
On Fri, Jul 26, 2019 at 06:36:01PM +0200, Maxime Ripard wrote:
> > +
> > + dvdd12-supply:
> > + maxItems: 1
> > + description: Regulator for 1.2V digital core power.
> > + $ref: /schemas/types.yaml#/definitions/phandle
> > +
> > + dvdd25-supply:
> > + maxItems: 1
> > + description: Regulator for 2.5V digital core power.
> > + $ref: /schemas/types.yaml#/definitions/phandle
>
> There's no need to specify the type here, all the properties ending in
> -supply are already checked for that type
Ok, thanks for the hint.
> > + ports:
> > + type: object
> > + minItems: 1
> > + maxItems: 2
> > + description: |
> > + Video port 0 for LVTTL input,
> > + Video port 1 for eDP output (panel or connector)
> > + using the DT bindings defined in
> > + Documentation/devicetree/bindings/media/video-interfaces.txt
>
> You should probably describe the port@0 and port@1 nodes here as
> well. It would allow you to express that the port 0 is mandatory and
> the port 1 optional, which got dropped in the conversion.
I would have liked to, but have not discovered yet a comprehensive source
of information about recommended syntax and semantics of the YAML schemes.
Is there some central reference for these types of issues? I mean not the
"here is a git repo with the meta-schemes" but sort of a cookbook?
Torsten
^ permalink raw reply
* Re: [PATCH 2/4] net: phy: Add mdio-aspeed
From: Andrew Lunn @ 2019-07-29 13:32 UTC (permalink / raw)
To: Andrew Jeffery
Cc: netdev, davem, robh+dt, mark.rutland, joel, f.fainelli,
hkallweit1, devicetree, linux-arm-kernel, linux-aspeed,
linux-kernel
In-Reply-To: <20190729043926.32679-3-andrew@aj.id.au>
On Mon, Jul 29, 2019 at 02:09:24PM +0930, Andrew Jeffery wrote:
> The AST2600 design separates the MDIO controllers from the MAC, which is
> where they were placed in the AST2400 and AST2500. Further, the register
> interface is reworked again, so now we have three possible different
> interface implementations, however this driver only supports the
> interface provided by the AST2600. The AST2400 and AST2500 will continue
> to be supported by the MDIO support embedded in the FTGMAC100 driver.
>
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> ---
> drivers/net/phy/Kconfig | 13 +++
> drivers/net/phy/Makefile | 1 +
> drivers/net/phy/mdio-aspeed.c | 159 ++++++++++++++++++++++++++++++++++
> 3 files changed, 173 insertions(+)
> create mode 100644 drivers/net/phy/mdio-aspeed.c
>
> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> index 20f14c5fbb7e..206d8650ee7f 100644
> --- a/drivers/net/phy/Kconfig
> +++ b/drivers/net/phy/Kconfig
> @@ -21,6 +21,19 @@ config MDIO_BUS
>
> if MDIO_BUS
>
> +config MDIO_ASPEED
> + tristate "ASPEED MDIO bus controller"
> + depends on ARCH_ASPEED || COMPILE_TEST
> + depends on OF_MDIO && HAS_IOMEM
> + help
> + This module provides a driver for the independent MDIO bus
> + controllers found in the ASPEED AST2600 SoC. This is a driver for the
> + third revision of the ASPEED MDIO register interface - the first two
> + revisions are the "old" and "new" interfaces found in the AST2400 and
> + AST2500, embedded in the MAC. For legacy reasons, FTGMAC100 driver
> + continues to drive the embedded MDIO controller for the AST2400 and
> + AST2500 SoCs, so say N if AST2600 support is not required.
> +
> config MDIO_BCM_IPROC
> tristate "Broadcom iProc MDIO bus controller"
> depends on ARCH_BCM_IPROC || COMPILE_TEST
> diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
> index 839acb292c38..ba07c27e4208 100644
> --- a/drivers/net/phy/Makefile
> +++ b/drivers/net/phy/Makefile
> @@ -22,6 +22,7 @@ libphy-$(CONFIG_LED_TRIGGER_PHY) += phy_led_triggers.o
> obj-$(CONFIG_PHYLINK) += phylink.o
> obj-$(CONFIG_PHYLIB) += libphy.o
>
> +obj-$(CONFIG_MDIO_ASPEED) += mdio-aspeed.o
> obj-$(CONFIG_MDIO_BCM_IPROC) += mdio-bcm-iproc.o
> obj-$(CONFIG_MDIO_BCM_UNIMAC) += mdio-bcm-unimac.o
> obj-$(CONFIG_MDIO_BITBANG) += mdio-bitbang.o
> diff --git a/drivers/net/phy/mdio-aspeed.c b/drivers/net/phy/mdio-aspeed.c
> new file mode 100644
> index 000000000000..71496a9ff54a
> --- /dev/null
> +++ b/drivers/net/phy/mdio-aspeed.c
> @@ -0,0 +1,159 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/* Copyright (C) 2019 IBM Corp. */
> +
> +#include <linux/bitfield.h>
> +#include <linux/delay.h>
> +#include <linux/mdio.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_mdio.h>
> +#include <linux/phy.h>
> +#include <linux/platform_device.h>
> +
> +#define DRV_NAME "mdio-aspeed"
> +
> +#define ASPEED_MDIO_CTRL 0x0
> +#define ASPEED_MDIO_CTRL_FIRE BIT(31)
> +#define ASPEED_MDIO_CTRL_ST BIT(28)
> +#define ASPEED_MDIO_CTRL_ST_C45 0
> +#define ASPEED_MDIO_CTRL_ST_C22 1
> +#define ASPEED_MDIO_CTRL_OP GENMASK(27, 26)
> +#define MDIO_C22_OP_WRITE 0b01
> +#define MDIO_C22_OP_READ 0b10
> +#define ASPEED_MDIO_CTRL_PHYAD GENMASK(25, 21)
> +#define ASPEED_MDIO_CTRL_REGAD GENMASK(20, 16)
> +#define ASPEED_MDIO_CTRL_MIIWDATA GENMASK(15, 0)
> +
> +#define ASPEED_MDIO_DATA 0x4
> +#define ASPEED_MDIO_DATA_MDC_THRES GENMASK(31, 24)
> +#define ASPEED_MDIO_DATA_MDIO_EDGE BIT(23)
> +#define ASPEED_MDIO_DATA_MDIO_LATCH GENMASK(22, 20)
> +#define ASPEED_MDIO_DATA_IDLE BIT(16)
> +#define ASPEED_MDIO_DATA_MIIRDATA GENMASK(15, 0)
> +
> +#define ASPEED_MDIO_RETRIES 10
> +
> +struct aspeed_mdio {
> + void __iomem *base;
> +};
> +
> +static int aspeed_mdio_read(struct mii_bus *bus, int addr, int regnum)
> +{
> + struct aspeed_mdio *ctx = bus->priv;
> + u32 ctrl;
> + int i;
> +
> + dev_dbg(&bus->dev, "%s: addr: %d, regnum: %d\n", __func__, addr,
> + regnum);
> +
> + /* Just clause 22 for the moment */
> + ctrl = ASPEED_MDIO_CTRL_FIRE
Hi Andrew
In the binding, you say C45 is supported. Here you don't. It would be
nice to be consistent.
> + | FIELD_PREP(ASPEED_MDIO_CTRL_ST, ASPEED_MDIO_CTRL_ST_C22)
> + | FIELD_PREP(ASPEED_MDIO_CTRL_OP, MDIO_C22_OP_READ)
> + | FIELD_PREP(ASPEED_MDIO_CTRL_PHYAD, addr)
> + | FIELD_PREP(ASPEED_MDIO_CTRL_REGAD, regnum);
> +
> + iowrite32(ctrl, ctx->base + ASPEED_MDIO_CTRL);
> +
> + for (i = 0; i < ASPEED_MDIO_RETRIES; i++) {
> + u32 data;
> +
> + data = ioread32(ctx->base + ASPEED_MDIO_DATA);
> + if (data & ASPEED_MDIO_DATA_IDLE)
> + return FIELD_GET(ASPEED_MDIO_DATA_MIIRDATA, data);
> +
> + udelay(100);
> + }
One of the readx_poll_timeout functions could be used.
> +
> + dev_err(&bus->dev, "MDIO read failed\n");
> + return -EIO;
> +}
> +
> +static int aspeed_mdio_write(struct mii_bus *bus, int addr, int regnum, u16 val)
> +{
> + struct aspeed_mdio *ctx = bus->priv;
> + u32 ctrl;
> + int i;
> +
> + dev_dbg(&bus->dev, "%s: addr: %d, regnum: %d, val: 0x%x\n",
> + __func__, addr, regnum, val);
> +
> + /* Just clause 22 for the moment */
> + ctrl = ASPEED_MDIO_CTRL_FIRE
> + | FIELD_PREP(ASPEED_MDIO_CTRL_ST, ASPEED_MDIO_CTRL_ST_C22)
> + | FIELD_PREP(ASPEED_MDIO_CTRL_OP, MDIO_C22_OP_WRITE)
> + | FIELD_PREP(ASPEED_MDIO_CTRL_PHYAD, addr)
> + | FIELD_PREP(ASPEED_MDIO_CTRL_REGAD, regnum)
> + | FIELD_PREP(ASPEED_MDIO_CTRL_MIIWDATA, val);
> +
> + iowrite32(ctrl, ctx->base + ASPEED_MDIO_CTRL);
> +
> + for (i = 0; i < ASPEED_MDIO_RETRIES; i++) {
> + ctrl = ioread32(ctx->base + ASPEED_MDIO_CTRL);
> + if (!(ctrl & ASPEED_MDIO_CTRL_FIRE))
> + return 0;
> +
> + udelay(100);
> + }
readx_poll_timeout() here as well.
Otherwise this looks good.
Andrew
^ permalink raw reply
* Re: [PATCH 1/3] dt-bindings: media: amlogic,vdec: add default compatible
From: Neil Armstrong @ 2019-07-29 13:06 UTC (permalink / raw)
To: Maxime Jourdan, Kevin Hilman
Cc: linux-amlogic, linux-kernel, linux-arm-kernel, devicetree
In-Reply-To: <20190726124639.7713-2-mjourdan@baylibre.com>
On 26/07/2019 14:46, Maxime Jourdan wrote:
> The first version of the bindings is missing a generic compatible that
> is used by the base node (GX), and then extended by the SoC device trees
> (GXBB, GXL, GXM)
>
> Also change the example to use "video-codec" instead of "video-decoder",
> as the former is the one used in almost all cases when it comes to video
> decode/encode accelerators.
>
> Signed-off-by: Maxime Jourdan <mjourdan@baylibre.com>
> ---
> Documentation/devicetree/bindings/media/amlogic,vdec.txt | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/media/amlogic,vdec.txt b/Documentation/devicetree/bindings/media/amlogic,vdec.txt
> index aabdd01bcf32..9b6aace86ca7 100644
> --- a/Documentation/devicetree/bindings/media/amlogic,vdec.txt
> +++ b/Documentation/devicetree/bindings/media/amlogic,vdec.txt
> @@ -26,6 +26,7 @@ Required properties:
> - GXBB (S905) : "amlogic,gxbb-vdec"
> - GXL (S905X, S905D) : "amlogic,gxl-vdec"
> - GXM (S912) : "amlogic,gxm-vdec"
> + followed by the common "amlogic,gx-vdec"
> - reg: base address and size of he following memory-mapped regions :
> - dos
> - esparser
> @@ -47,8 +48,8 @@ Required properties:
>
> Example:
>
> -vdec: video-decoder@c8820000 {
> - compatible = "amlogic,gxbb-vdec";
> +vdec: video-codec@c8820000 {
> + compatible = "amlogic,gxbb-vdec", "amlogic,gx-vdec";
> reg = <0x0 0xc8820000 0x0 0x10000>,
> <0x0 0xc110a580 0x0 0xe4>;
> reg-names = "dos", "esparser";
>
Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
^ permalink raw reply
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