* [PATCH V2 3/4] ARM: imx_v6_v7_defconfig: Enable CONFIG_IMX7ULP_WDT by default
From: Anson.Huang @ 2019-08-12 8:53 UTC (permalink / raw)
To: wim, linux, robh+dt, mark.rutland, shawnguo, s.hauer, kernel,
festevam, linux, otavio, leonard.crestez, u.kleine-koenig,
schnitzeltony, jan.tuerk, linux-watchdog, devicetree,
linux-arm-kernel, linux-kernel
Cc: Linux-imx
In-Reply-To: <20190812085321.13823-1-Anson.Huang@nxp.com>
From: Anson Huang <Anson.Huang@nxp.com>
Select CONFIG_IMX7ULP_WDT by default to support i.MX7ULP watchdog.
Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
No changes.
---
arch/arm/configs/imx_v6_v7_defconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/configs/imx_v6_v7_defconfig b/arch/arm/configs/imx_v6_v7_defconfig
index bd2e2f5..f69075b 100644
--- a/arch/arm/configs/imx_v6_v7_defconfig
+++ b/arch/arm/configs/imx_v6_v7_defconfig
@@ -235,6 +235,7 @@ CONFIG_DA9062_WATCHDOG=y
CONFIG_DA9063_WATCHDOG=m
CONFIG_RN5T618_WATCHDOG=y
CONFIG_IMX2_WDT=y
+CONFIG_IMX7ULP_WDT=y
CONFIG_MFD_DA9052_I2C=y
CONFIG_MFD_DA9062=y
CONFIG_MFD_DA9063=y
--
2.7.4
^ permalink raw reply related
* [PATCH V2 4/4] ARM: dts: imx7ulp: Add wdog1 node
From: Anson.Huang @ 2019-08-12 8:53 UTC (permalink / raw)
To: wim, linux, robh+dt, mark.rutland, shawnguo, s.hauer, kernel,
festevam, linux, otavio, leonard.crestez, u.kleine-koenig,
schnitzeltony, jan.tuerk, linux-watchdog, devicetree,
linux-arm-kernel, linux-kernel
Cc: Linux-imx
In-Reply-To: <20190812085321.13823-1-Anson.Huang@nxp.com>
From: Anson Huang <Anson.Huang@nxp.com>
Add wdog1 node to support watchdog driver.
Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
No changes.
---
arch/arm/boot/dts/imx7ulp.dtsi | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/arch/arm/boot/dts/imx7ulp.dtsi b/arch/arm/boot/dts/imx7ulp.dtsi
index 6859a3a..1fdb5a35 100644
--- a/arch/arm/boot/dts/imx7ulp.dtsi
+++ b/arch/arm/boot/dts/imx7ulp.dtsi
@@ -264,6 +264,16 @@
#clock-cells = <1>;
};
+ wdog1: wdog@403d0000 {
+ compatible = "fsl,imx7ulp-wdt";
+ reg = <0x403d0000 0x10000>;
+ interrupts = <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&pcc2 IMX7ULP_CLK_WDG1>;
+ assigned-clocks = <&pcc2 IMX7ULP_CLK_WDG1>;
+ assigned-clocks-parents = <&scg1 IMX7ULP_CLK_FIRC_BUS_CLK>;
+ timeout-sec = <40>;
+ };
+
pcc2: clock-controller@403f0000 {
compatible = "fsl,imx7ulp-pcc2";
reg = <0x403f0000 0x10000>;
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v3 17/21] ARM: dts: imx6ull: improve can templates
From: Philippe Schenker @ 2019-08-12 8:54 UTC (permalink / raw)
To: stefan@agner.ch, Marcel Ziswiler, festevam@gmail.com,
devicetree@vger.kernel.org, mark.rutland@arm.com,
Max Krummenacher, shawnguo@kernel.org, michal.vokac@ysoft.com,
robh+dt@kernel.org
Cc: linux-arm-kernel@lists.infradead.org, s.hauer@pengutronix.de,
kernel@pengutronix.de, linux-kernel@vger.kernel.org,
linux-imx@nxp.com
In-Reply-To: <8ae5742f29f17e61fd9fc39a8dbd1b7c3a2f45b0.camel@toradex.com>
On Fri, 2019-08-09 at 15:47 +0000, Marcel Ziswiler wrote:
> Hi Philippe
>
> On Wed, 2019-08-07 at 08:26 +0000, Philippe Schenker wrote:
> > From: Max Krummenacher <max.krummenacher@toradex.com>
> >
> > Add the pinmuxing and a inactive node for flexcan1 on SODIMM 55/63
> > and move the inactive flexcan nodes to imx6ull-colibri-eval-v3.dtsi
> > where they belong.
> >
> > Note that this commit does not enable flexcan functionality, but
> > rather
> > eases the effort needed to do so.
> >
> > Signed-off-by: Max Krummenacher <max.krummenacher@toradex.com>
> > Signed-off-by: Philippe Schenker <philippe.schenker@toradex.com>
> > ---
> >
> > Changes in v3: None
> > Changes in v2: None
> >
> > arch/arm/boot/dts/imx6ull-colibri-eval-v3.dtsi | 12 ++++++++++++
> > arch/arm/boot/dts/imx6ull-colibri-nonwifi.dtsi | 2 +-
> > arch/arm/boot/dts/imx6ull-colibri-wifi.dtsi | 2 +-
> > arch/arm/boot/dts/imx6ull-colibri.dtsi | 16 ++++++++++++++-
> > -
> > 4 files changed, 28 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/arm/boot/dts/imx6ull-colibri-eval-v3.dtsi
> > b/arch/arm/boot/dts/imx6ull-colibri-eval-v3.dtsi
> > index b6147c76d159..3bee37c75aa6 100644
> > --- a/arch/arm/boot/dts/imx6ull-colibri-eval-v3.dtsi
> > +++ b/arch/arm/boot/dts/imx6ull-colibri-eval-v3.dtsi
> > @@ -83,6 +83,18 @@
> > };
> > };
> >
> > +&can1 {
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&pinctrl_flexcan1>;
> > + status = "disabled";
> > +};
> > +
> > +&can2 {
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&pinctrl_flexcan2>;
> > + status = "disabled";
> > +};
>
> As those don't really have anything to do with the eval board
> directly,
> wouldn't it make more sense to rather move them into the module's dtsi
> just like the pin muxing further below?
I agree, moved for v4.
Thanks, Philippe
>
> > &i2c1 {
> > status = "okay";
> >
> > diff --git a/arch/arm/boot/dts/imx6ull-colibri-nonwifi.dtsi
> > b/arch/arm/boot/dts/imx6ull-colibri-nonwifi.dtsi
> > index fb213bec4654..95a11b8bcbdb 100644
> > --- a/arch/arm/boot/dts/imx6ull-colibri-nonwifi.dtsi
> > +++ b/arch/arm/boot/dts/imx6ull-colibri-nonwifi.dtsi
> > @@ -15,7 +15,7 @@
> > &iomuxc {
> > pinctrl-names = "default";
> > pinctrl-0 = <&pinctrl_gpio1 &pinctrl_gpio2 &pinctrl_gpio3
> > - &pinctrl_gpio4 &pinctrl_gpio5 &pinctrl_gpio6>;
> > + &pinctrl_gpio4 &pinctrl_gpio5 &pinctrl_gpio6
> > &pinctrl_gpio7>;
> > };
> >
> > &iomuxc_snvs {
> > diff --git a/arch/arm/boot/dts/imx6ull-colibri-wifi.dtsi
> > b/arch/arm/boot/dts/imx6ull-colibri-wifi.dtsi
> > index 038d8c90f6df..a0545431b3dc 100644
> > --- a/arch/arm/boot/dts/imx6ull-colibri-wifi.dtsi
> > +++ b/arch/arm/boot/dts/imx6ull-colibri-wifi.dtsi
> > @@ -26,7 +26,7 @@
> > &iomuxc {
> > pinctrl-names = "default";
> > pinctrl-0 = <&pinctrl_gpio1 &pinctrl_gpio2 &pinctrl_gpio3
> > - &pinctrl_gpio4 &pinctrl_gpio5>;
> > + &pinctrl_gpio4 &pinctrl_gpio5 &pinctrl_gpio7>;
> >
> > };
> >
> > diff --git a/arch/arm/boot/dts/imx6ull-colibri.dtsi
> > b/arch/arm/boot/dts/imx6ull-colibri.dtsi
> > index e3220298dd6f..553d4c1f80e9 100644
> > --- a/arch/arm/boot/dts/imx6ull-colibri.dtsi
> > +++ b/arch/arm/boot/dts/imx6ull-colibri.dtsi
> > @@ -256,6 +256,13 @@
> > >;
> > };
> >
> > + pinctrl_flexcan1: flexcan1-grp {
> > + fsl,pins = <
> > + MX6UL_PAD_ENET1_RX_DATA0__FLEXCAN1_TX 0x1b
> > 0
> > 20
> > + MX6UL_PAD_ENET1_RX_DATA1__FLEXCAN1_RX 0x1b
> > 0
> > 20
> > + >;
> > + };
> > +
> > pinctrl_flexcan2: flexcan2-grp {
> > fsl,pins = <
> > MX6UL_PAD_ENET1_TX_DATA0__FLEXCAN2_RX 0x1b
> > 0
> > 20
> > @@ -271,8 +278,6 @@
> >
> > pinctrl_gpio1: gpio1-grp {
> > fsl,pins = <
> > - MX6UL_PAD_ENET1_RX_DATA0__GPIO2_IO00 0x74
> > /* SODIMM 55 */
> > - MX6UL_PAD_ENET1_RX_DATA1__GPIO2_IO01 0x74
> > /* SODIMM 63 */
> > MX6UL_PAD_UART3_RX_DATA__GPIO1_IO25 0X14
> > /* SODIMM 77 */
> > MX6UL_PAD_JTAG_TCK__GPIO1_IO14 0x14
> > /* SODIMM 99 */
> > MX6UL_PAD_NAND_CE1_B__GPIO4_IO14 0x14 /*
> > SODIMM 133 */
> > @@ -325,6 +330,13 @@
> > >;
> > };
> >
> > + pinctrl_gpio7: gpio7-grp { /* CAN1 */
> > + fsl,pins = <
> > + MX6UL_PAD_ENET1_RX_DATA0__GPIO2_IO00 0x74
> > /* SODIMM 55 */
> > + MX6UL_PAD_ENET1_RX_DATA1__GPIO2_IO01 0x74
> > /* SODIMM 63 */
> > + >;
> > + };
> > +
> > pinctrl_gpmi_nand: gpmi-nand-grp {
> > fsl,pins = <
> > MX6UL_PAD_NAND_DATA00__RAWNAND_DATA00 0x10
> > 0
> > a9
>
> Cheers
>
> Marcel
^ permalink raw reply
* Re: [PATCH v5 02/18] dt-bindings: thermal: add binding document for h6 thermal controller
From: Maxime Ripard @ 2019-08-12 8:56 UTC (permalink / raw)
To: Yangtao Li
Cc: rui.zhang, edubezval, daniel.lezcano, robh+dt, mark.rutland, wens,
mchehab+samsung, davem, gregkh, Jonathan.Cameron, nicolas.ferre,
linux-pm, devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <20190810052829.6032-3-tiny.windzz@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2562 bytes --]
On Sat, Aug 10, 2019 at 05:28:13AM +0000, Yangtao Li wrote:
> This patch adds binding document for allwinner h6 thermal controller.
>
> Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
> ---
> .../bindings/thermal/sun8i-thermal.yaml | 79 +++++++++++++++++++
> 1 file changed, 79 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/thermal/sun8i-thermal.yaml
>
> diff --git a/Documentation/devicetree/bindings/thermal/sun8i-thermal.yaml b/Documentation/devicetree/bindings/thermal/sun8i-thermal.yaml
> new file mode 100644
> index 000000000000..e0973199ba3c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/thermal/sun8i-thermal.yaml
We've used so far for the schemas the first compatible to introduce
that controller as the filename, we should be consistent here. In that
case that would be allwinner,sun8i-a23-ths.yaml
> @@ -0,0 +1,79 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/thermal/sun8i-thermal.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Allwinner SUN8I Thermal Controller Device Tree Bindings
> +
> +maintainers:
> + - Yangtao Li <tiny.windzz@gmail.com>
> +
> +description: |-
> + This describes the device tree binding for the Allwinner thermal
> + controller which measures the on-SoC temperatures.
> +
> +properties:
> + compatible:
> + enum:
> + - allwinner,sun50i-h6-ths
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + resets:
> + maxItems: 1
> +
> + clocks:
> + maxItems: 1
> +
> + clock-names:
> + const: bus
> +
> + "#thermal-sensor-cells":
> + const: 1
> +
> + nvmem-cells:
You need a maxItems here too
> + description: ths calibrate data
What about something like this:
Calibration data for the thermal sensor
> +
> + nvmem-cell-names:
> + const: calib
I'm not sure we need a abbreviation here, calibration would be more
explicit
> +
> +required:
> + - compatible
> + - reg
> + - reset
> + - clocks
> + - clock-names
> + - interrupts
> + - "#thermal-sensor-cells"
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + ths: ths@5070400 {
> + compatible = "allwinner,sun50i-h6-ths";
> + reg = <0x05070400 0x100>;
> + clocks = <&ccu CLK_BUS_THS>;
> + clock-names = "bus";
> + resets = <&ccu RST_BUS_THS>;
> + interrupts = <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>;
Those examples won't compile.
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
* Re: [PATCH 03/11] clocksource: sun4i: Add missing compatibles
From: Maxime Ripard @ 2019-08-12 8:58 UTC (permalink / raw)
To: daniel.lezcano, tglx
Cc: Mark Rutland, devicetree, Chen-Yu Tsai, Rob Herring, Frank Rowand,
linux-arm-kernel
In-Reply-To: <20190722081229.22422-3-maxime.ripard@bootlin.com>
[-- Attachment #1.1: Type: text/plain, Size: 382 bytes --]
Hi Daniel, Thomas,
On Mon, Jul 22, 2019 at 10:12:21AM +0200, Maxime Ripard wrote:
> Newer Allwinner SoCs have different number of interrupts, let's add
> different compatibles for all of them to deal with this properly.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
Ping?
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
[-- Attachment #2: 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 03/11] clocksource: sun4i: Add missing compatibles
From: Daniel Lezcano @ 2019-08-12 8:59 UTC (permalink / raw)
To: Maxime Ripard, Mark Rutland, Rob Herring, Frank Rowand,
Chen-Yu Tsai, tglx
Cc: devicetree, linux-arm-kernel
In-Reply-To: <20190722081229.22422-3-maxime.ripard@bootlin.com>
On 22/07/2019 10:12, Maxime Ripard wrote:
> Newer Allwinner SoCs have different number of interrupts, let's add
> different compatibles for all of them to deal with this properly.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
> drivers/clocksource/timer-sun4i.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/clocksource/timer-sun4i.c b/drivers/clocksource/timer-sun4i.c
> index 65f38f6ca714..0ba8155b8287 100644
> --- a/drivers/clocksource/timer-sun4i.c
> +++ b/drivers/clocksource/timer-sun4i.c
> @@ -219,5 +219,9 @@ static int __init sun4i_timer_init(struct device_node *node)
> }
> TIMER_OF_DECLARE(sun4i, "allwinner,sun4i-a10-timer",
> sun4i_timer_init);
> +TIMER_OF_DECLARE(sun8i_a23, "allwinner,sun8i-a23-timer",
> + sun4i_timer_init);
> +TIMER_OF_DECLARE(sun8i_v3s, "allwinner,sun8i-v3s-timer",
> + sun4i_timer_init);
> TIMER_OF_DECLARE(suniv, "allwinner,suniv-f1c100s-timer",
> sun4i_timer_init);
>
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
_______________________________________________
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/4] staging: iio: adc: ad7192: Add low_pass_3db_filter_frequency
From: Mircea Caprioru @ 2019-08-12 9:03 UTC (permalink / raw)
To: jic23
Cc: Michael.Hennerich, stefan.popa, lars, gregkh, linux-kernel,
linux-iio, devicetree, robh+dt, Mircea Caprioru
By adding this option we are able to remove the sync3 field and dt binding.
When setting the required cutoff frequency we also determine the ADC
configuration for chop and sync filter.
Signed-off-by: Mircea Caprioru <mircea.caprioru@analog.com>
---
drivers/staging/iio/adc/ad7192.c | 148 +++++++++++++++++++++++++++----
1 file changed, 132 insertions(+), 16 deletions(-)
diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/ad7192.c
index 81ea2639c67c..d58ce08f3693 100644
--- a/drivers/staging/iio/adc/ad7192.c
+++ b/drivers/staging/iio/adc/ad7192.c
@@ -143,6 +143,10 @@
#define AD7192_EXT_FREQ_MHZ_MAX 5120000
#define AD7192_INT_FREQ_MHZ 4915200
+#define AD7192_NO_SYNC_FILTER 1
+#define AD7192_SYNC3_FILTER 3
+#define AD7192_SYNC4_FILTER 4
+
/* NOTE:
* The AD7190/2/5 features a dual use data out ready DOUT/RDY output.
* In order to avoid contentions on the SPI bus, it's therefore necessary
@@ -250,7 +254,7 @@ static int ad7192_of_clock_select(struct ad7192_state *st)
static int ad7192_setup(struct ad7192_state *st, struct device_node *np)
{
struct iio_dev *indio_dev = spi_get_drvdata(st->sd.spi);
- bool rej60_en, sinc3_en, refin2_en, chop_en;
+ bool rej60_en, refin2_en;
bool buf_en, bipolar, burnout_curr_en;
unsigned long long scale_uv;
int i, ret, id;
@@ -282,24 +286,12 @@ static int ad7192_setup(struct ad7192_state *st, struct device_node *np)
if (rej60_en)
st->mode |= AD7192_MODE_REJ60;
- sinc3_en = of_property_read_bool(np, "adi,sinc3-filter-enable");
- if (sinc3_en)
- st->mode |= AD7192_MODE_SINC3;
-
refin2_en = of_property_read_bool(np, "adi,refin2-pins-enable");
if (refin2_en && st->devid != ID_AD7195)
st->conf |= AD7192_CONF_REFSEL;
- chop_en = of_property_read_bool(np, "adi,chop-enable");
- if (chop_en) {
- st->conf |= AD7192_CONF_CHOP;
- if (sinc3_en)
- st->f_order = 3; /* SINC 3rd order */
- else
- st->f_order = 4; /* SINC 4th order */
- } else {
- st->f_order = 1;
- }
+ st->conf &= ~AD7192_CONF_CHOP;
+ st->f_order = AD7192_NO_SYNC_FILTER;
buf_en = of_property_read_bool(np, "adi,buffer-enable");
if (buf_en)
@@ -311,7 +303,7 @@ static int ad7192_setup(struct ad7192_state *st, struct device_node *np)
burnout_curr_en = of_property_read_bool(np,
"adi,burnout-currents-enable");
- if (burnout_curr_en && buf_en && !chop_en) {
+ if (burnout_curr_en && buf_en) {
st->conf |= AD7192_CONF_BURN;
} else if (burnout_curr_en) {
dev_warn(&st->sd.spi->dev,
@@ -409,6 +401,49 @@ static ssize_t ad7192_set(struct device *dev,
return ret ? ret : len;
}
+static void ad7192_get_available_filter_freq(struct ad7192_state *st,
+ int *freq)
+{
+ unsigned int fadc;
+
+ /* Formulas for filter at page 25 of the datasheet */
+ fadc = DIV_ROUND_CLOSEST(st->fclk,
+ AD7192_SYNC4_FILTER * AD7192_MODE_RATE(st->mode));
+ freq[0] = DIV_ROUND_CLOSEST(fadc * 240, 1024);
+
+ fadc = DIV_ROUND_CLOSEST(st->fclk,
+ AD7192_SYNC3_FILTER * AD7192_MODE_RATE(st->mode));
+ freq[1] = DIV_ROUND_CLOSEST(fadc * 240, 1024);
+
+ fadc = DIV_ROUND_CLOSEST(st->fclk, AD7192_MODE_RATE(st->mode));
+ freq[2] = DIV_ROUND_CLOSEST(fadc * 230, 1024);
+ freq[3] = DIV_ROUND_CLOSEST(fadc * 272, 1024);
+}
+
+static int ad7192_show_filter_avail(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+ struct ad7192_state *st = iio_priv(indio_dev);
+ unsigned int freq_avail[4], i;
+ size_t len = 0;
+
+ ad7192_get_available_filter_freq(st, freq_avail);
+
+ for (i = 0; i < ARRAY_SIZE(freq_avail); i++)
+ len += scnprintf(buf + len, PAGE_SIZE - len,
+ "%d.%d ", freq_avail[i] / 1000,
+ freq_avail[i] % 1000);
+
+ buf[len - 1] = '\n';
+
+ return len;
+}
+
+static IIO_DEVICE_ATTR(filter_low_pass_3db_frequency_available,
+ 0444, ad7192_show_filter_avail, NULL, 0);
+
static IIO_DEVICE_ATTR(bridge_switch_en, 0644,
ad7192_show_bridge_switch, ad7192_set,
AD7192_REG_GPOCON);
@@ -418,6 +453,7 @@ static IIO_DEVICE_ATTR(ac_excitation_en, 0644,
AD7192_REG_MODE);
static struct attribute *ad7192_attributes[] = {
+ &iio_dev_attr_filter_low_pass_3db_frequency_available.dev_attr.attr,
&iio_dev_attr_bridge_switch_en.dev_attr.attr,
&iio_dev_attr_ac_excitation_en.dev_attr.attr,
NULL
@@ -428,6 +464,7 @@ static const struct attribute_group ad7192_attribute_group = {
};
static struct attribute *ad7195_attributes[] = {
+ &iio_dev_attr_filter_low_pass_3db_frequency_available.dev_attr.attr,
&iio_dev_attr_bridge_switch_en.dev_attr.attr,
NULL
};
@@ -441,6 +478,74 @@ static unsigned int ad7192_get_temp_scale(bool unipolar)
return unipolar ? 2815 * 2 : 2815;
}
+static int ad7192_set_3db_filter_freq(struct ad7192_state *st,
+ int val, int val2)
+{
+ int freq_avail[4], i, ret, idx, freq;
+ unsigned int diff_new, diff_old;
+
+ diff_old = U32_MAX;
+ freq = val * 1000 + val2;
+
+ ad7192_get_available_filter_freq(st, freq_avail);
+
+ for (i = 0; i < ARRAY_SIZE(freq_avail); i++) {
+ diff_new = abs(freq - freq_avail[i]);
+ if (diff_new < diff_old) {
+ diff_old = diff_new;
+ idx = i;
+ }
+ }
+
+ switch (idx) {
+ case 0:
+ st->f_order = AD7192_SYNC4_FILTER;
+ st->mode &= ~AD7192_MODE_SINC3;
+
+ st->conf |= AD7192_CONF_CHOP;
+ break;
+ case 1:
+ st->f_order = AD7192_SYNC3_FILTER;
+ st->mode |= AD7192_MODE_SINC3;
+
+ st->conf |= AD7192_CONF_CHOP;
+ break;
+ case 2:
+ st->f_order = AD7192_NO_SYNC_FILTER;
+ st->mode &= ~AD7192_MODE_SINC3;
+
+ st->conf &= ~AD7192_CONF_CHOP;
+ break;
+ case 3:
+ st->f_order = AD7192_NO_SYNC_FILTER;
+ st->mode |= AD7192_MODE_SINC3;
+
+ st->conf &= ~AD7192_CONF_CHOP;
+ break;
+ }
+
+ ret = ad_sd_write_reg(&st->sd, AD7192_REG_MODE, 3, st->mode);
+ if (ret < 0)
+ return ret;
+
+ return ad_sd_write_reg(&st->sd, AD7192_REG_CONF, 3, st->conf);
+}
+
+static int ad7192_get_3db_filter_freq(struct ad7192_state *st)
+{
+ unsigned int fadc;
+
+ fadc = DIV_ROUND_CLOSEST(st->fclk,
+ st->f_order * AD7192_MODE_RATE(st->mode));
+
+ if (st->conf & AD7192_CONF_CHOP)
+ return DIV_ROUND_CLOSEST(fadc * 240, 1024);
+ if (st->mode & AD7192_MODE_SINC3)
+ return DIV_ROUND_CLOSEST(fadc * 272, 1024);
+ else
+ return DIV_ROUND_CLOSEST(fadc * 230, 1024);
+}
+
static int ad7192_read_raw(struct iio_dev *indio_dev,
struct iio_chan_spec const *chan,
int *val,
@@ -481,6 +586,10 @@ static int ad7192_read_raw(struct iio_dev *indio_dev,
*val = st->fclk /
(st->f_order * 1024 * AD7192_MODE_RATE(st->mode));
return IIO_VAL_INT;
+ case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
+ *val = ad7192_get_3db_filter_freq(st);
+ *val2 = 1000;
+ return IIO_VAL_FRACTIONAL;
}
return -EINVAL;
@@ -535,6 +644,9 @@ static int ad7192_write_raw(struct iio_dev *indio_dev,
st->mode |= AD7192_MODE_RATE(div);
ad_sd_write_reg(&st->sd, AD7192_REG_MODE, 3, st->mode);
break;
+ case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
+ ret = ad7192_set_3db_filter_freq(st, val, val2 / 1000);
+ break;
default:
ret = -EINVAL;
}
@@ -553,6 +665,8 @@ static int ad7192_write_raw_get_fmt(struct iio_dev *indio_dev,
return IIO_VAL_INT_PLUS_NANO;
case IIO_CHAN_INFO_SAMP_FREQ:
return IIO_VAL_INT;
+ case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
+ return IIO_VAL_INT_PLUS_MICRO;
default:
return -EINVAL;
}
@@ -653,6 +767,8 @@ static int ad7192_channels_config(struct iio_dev *indio_dev)
for (i = 0; i < indio_dev->num_channels; i++) {
*chan = channels[i];
+ chan->info_mask_shared_by_all |=
+ BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY);
if (chan->type != IIO_TEMP)
chan->info_mask_shared_by_type_available |=
BIT(IIO_CHAN_INFO_SCALE);
--
2.17.1
^ permalink raw reply related
* [PATCH 2/4] iio: adc: ad_sigma_delta: Export ad_sd_calibrate
From: Mircea Caprioru @ 2019-08-12 9:03 UTC (permalink / raw)
To: jic23
Cc: Michael.Hennerich, stefan.popa, lars, gregkh, linux-kernel,
linux-iio, devicetree, robh+dt, Mircea Caprioru
In-Reply-To: <20190812090341.27183-1-mircea.caprioru@analog.com>
This patch exports the ad_sd_calibrate function in order to be able to
call it from outside ad_sigma_delta.
There are cases where the option to calibrate one channel at a time is
necessary (ex. system calibration for zero scale and full scale).
Signed-off-by: Mircea Caprioru <mircea.caprioru@analog.com>
---
drivers/iio/adc/ad_sigma_delta.c | 3 ++-
include/linux/iio/adc/ad_sigma_delta.h | 2 ++
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/iio/adc/ad_sigma_delta.c b/drivers/iio/adc/ad_sigma_delta.c
index 2640b75fb774..8ba90486c787 100644
--- a/drivers/iio/adc/ad_sigma_delta.c
+++ b/drivers/iio/adc/ad_sigma_delta.c
@@ -205,7 +205,7 @@ int ad_sd_reset(struct ad_sigma_delta *sigma_delta,
}
EXPORT_SYMBOL_GPL(ad_sd_reset);
-static int ad_sd_calibrate(struct ad_sigma_delta *sigma_delta,
+int ad_sd_calibrate(struct ad_sigma_delta *sigma_delta,
unsigned int mode, unsigned int channel)
{
int ret;
@@ -242,6 +242,7 @@ static int ad_sd_calibrate(struct ad_sigma_delta *sigma_delta,
return ret;
}
+EXPORT_SYMBOL_GPL(ad_sd_calibrate);
/**
* ad_sd_calibrate_all() - Performs channel calibration
diff --git a/include/linux/iio/adc/ad_sigma_delta.h b/include/linux/iio/adc/ad_sigma_delta.h
index 7716fa0c9fce..8a4e25a7080c 100644
--- a/include/linux/iio/adc/ad_sigma_delta.h
+++ b/include/linux/iio/adc/ad_sigma_delta.h
@@ -119,6 +119,8 @@ int ad_sd_reset(struct ad_sigma_delta *sigma_delta,
int ad_sigma_delta_single_conversion(struct iio_dev *indio_dev,
const struct iio_chan_spec *chan, int *val);
+int ad_sd_calibrate(struct ad_sigma_delta *sigma_delta,
+ unsigned int mode, unsigned int channel);
int ad_sd_calibrate_all(struct ad_sigma_delta *sigma_delta,
const struct ad_sd_calib_data *cd, unsigned int n);
int ad_sd_init(struct ad_sigma_delta *sigma_delta, struct iio_dev *indio_dev,
--
2.17.1
^ permalink raw reply related
* [PATCH 3/4] staging: iio: adc: ad7192: Add system calibration support
From: Mircea Caprioru @ 2019-08-12 9:03 UTC (permalink / raw)
To: jic23
Cc: Michael.Hennerich, stefan.popa, lars, gregkh, linux-kernel,
linux-iio, devicetree, robh+dt, Mircea Caprioru
In-Reply-To: <20190812090341.27183-1-mircea.caprioru@analog.com>
This patch will add a system calibration attribute for each channel. Using
this option the user will have the ability to calibrate each channel for
zero scale and full scale. It uses the iio_chan_spec_ext_info and IIO_ENUM
to implement the functionality.
Signed-off-by: Mircea Caprioru <mircea.caprioru@analog.com>
---
drivers/staging/iio/adc/ad7192.c | 55 +++++++++++++++++++++++++++++++-
1 file changed, 54 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/ad7192.c
index d58ce08f3693..731072830f30 100644
--- a/drivers/staging/iio/adc/ad7192.c
+++ b/drivers/staging/iio/adc/ad7192.c
@@ -155,6 +155,11 @@
* The DOUT/RDY output must also be wired to an interrupt capable GPIO.
*/
+enum {
+ AD7192_SYSCALIB_ZERO_SCALE,
+ AD7192_SYSCALIB_FULL_SCALE,
+};
+
struct ad7192_state {
struct regulator *avdd;
struct regulator *dvdd;
@@ -169,10 +174,56 @@ struct ad7192_state {
u8 devid;
u8 clock_sel;
struct mutex lock; /* protect sensor state */
+ u8 syscalib_mode[8];
struct ad_sigma_delta sd;
};
+static const char * const ad7192_syscalib_modes[] = {
+ [AD7192_SYSCALIB_ZERO_SCALE] = "zero_scale",
+ [AD7192_SYSCALIB_FULL_SCALE] = "full_scale",
+};
+
+static int ad7192_set_syscalib_mode(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan,
+ unsigned int mode)
+{
+ struct ad7192_state *st = iio_priv(indio_dev);
+ int ret;
+
+ st->syscalib_mode[chan->channel] = mode;
+
+ if (mode == AD7192_SYSCALIB_ZERO_SCALE)
+ ret = ad_sd_calibrate(&st->sd, AD7192_MODE_CAL_SYS_ZERO,
+ chan->address);
+ else
+ ret = ad_sd_calibrate(&st->sd, AD7192_MODE_CAL_SYS_FULL,
+ chan->address);
+
+ return ret;
+}
+
+static int ad7192_get_syscalib_mode(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan)
+{
+ struct ad7192_state *st = iio_priv(indio_dev);
+
+ return st->syscalib_mode[chan->channel];
+}
+
+static const struct iio_enum ad7192_syscalib_mode_enum = {
+ .items = ad7192_syscalib_modes,
+ .num_items = ARRAY_SIZE(ad7192_syscalib_modes),
+ .set = ad7192_set_syscalib_mode,
+ .get = ad7192_get_syscalib_mode
+};
+
+static const struct iio_chan_spec_ext_info ad7192_calibsys_ext_info[] = {
+ IIO_ENUM("system_calibration", IIO_SEPARATE, &ad7192_syscalib_mode_enum),
+ IIO_ENUM_AVAILABLE("system_calibration", &ad7192_syscalib_mode_enum),
+ {}
+};
+
static struct ad7192_state *ad_sigma_delta_to_ad7192(struct ad_sigma_delta *sd)
{
return container_of(sd, struct ad7192_state, sd);
@@ -769,9 +820,11 @@ static int ad7192_channels_config(struct iio_dev *indio_dev)
*chan = channels[i];
chan->info_mask_shared_by_all |=
BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY);
- if (chan->type != IIO_TEMP)
+ if (chan->type != IIO_TEMP) {
chan->info_mask_shared_by_type_available |=
BIT(IIO_CHAN_INFO_SCALE);
+ chan->ext_info = ad7192_calibsys_ext_info;
+ }
chan++;
}
--
2.17.1
^ permalink raw reply related
* [PATCH 4/4] dt-bindings: iio: adc: ad7192: Add binding documentation for AD7192
From: Mircea Caprioru @ 2019-08-12 9:03 UTC (permalink / raw)
To: jic23
Cc: Michael.Hennerich, stefan.popa, lars, gregkh, linux-kernel,
linux-iio, devicetree, robh+dt, Mircea Caprioru
In-Reply-To: <20190812090341.27183-1-mircea.caprioru@analog.com>
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="y", Size: 3825 bytes --]
This patch add device tree binding documentation for AD7192 adc in YAML
format.
Signed-off-by: Mircea Caprioru <mircea.caprioru@analog.com>
---
.../bindings/iio/adc/adi,ad7192.yaml | 123 ++++++++++++++++++
1 file changed, 123 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml
diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml
new file mode 100644
index 000000000000..a56ee391f6a8
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml
@@ -0,0 +1,123 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2019 Analog Devices Inc.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/bindings/iio/adc/adi,ad7192.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices AD7192 ADC device driver
+
+maintainers:
+ - Michael Hennerich <michael.hennerich@analog.com>
+
+description: |
+ Bindings for the Analog Devices AD7192 ADC device. Datasheet can be
+ found here:
+ https://www.analog.com/media/en/technical-documentation/data-sheets/AD7192.pdf
+
+properties:
+ compatible:
+ enum:
+ - adi,ad7190
+ - adi,ad7192
+ - adi,ad7193
+ - adi,ad7195
+
+ reg:
+ description: SPI chip select number for the device
+ maxItems: 1
+
+ spi-cpol: true
+
+ spi-cpha: true
+
+ clocks:
+ maxItems: 1
+ description: phandle to the master clock (mclk)
+
+ clock-names:
+ items:
+ - const: mclk
+
+ interrupts:
+ description: IRQ line for the ADC
+ maxItems: 1
+
+ dvdd-supply:
+ description: DVdd voltage supply
+ items:
+ - const: dvdd
+
+ avdd-supply:
+ description: AVdd voltage supply
+ items:
+ - const: avdd
+
+ adi,rejection-60-Hz-enable:
+ description: |
+ This bit enables a notch at 60 Hz when the first notch of the sinc
+ filter is at 50 Hz. When REJ60 is set, a filter notch is placed at
+ 60 Hz when the sinc filter first notch is at 50 Hz. This allows
+ simultaneous 50 Hz/ 60 Hz rejection.
+ type: boolean
+
+ adi,refin2-pins-enable:
+ description: |
+ External reference applied between the P1/REFIN2(+) and P0/REFIN2(−) pins.
+ type: boolean
+
+ adi,buffer-enable:
+ description: |
+ Enables the buffer on the analog inputs. If cleared, the analog inputs
+ are unbuffered, lowering the power consumption of the device. If this
+ bit is set, the analog inputs are buffered, allowing the user to place
+ source impedances on the front end without contributing gain errors to
+ the system.
+ type: boolean
+
+ adi,burnout-currents-enable:
+ description: |
+ When this bit is set to 1, the 500 nA current sources in the signal
+ path are enabled. When BURN = 0, the burnout currents are disabled.
+ The burnout currents can be enabled only when the buffer is active
+ and when chop is disabled.
+ type: boolean
+
+ bipolar:
+ description: see Documentation/devicetree/bindings/iio/adc/adc.txt
+ type: boolean
+
+required:
+ - compatible
+ - reg
+ - clocks
+ - clock-names
+ - interrupts
+ - dvdd-supply
+ - avdd-supply
+ - spi-cpol
+ - spi-cpha
+
+examples:
+ - |
+ spi0 {
+ ad7192@0 {
+ compatible = "adi,ad7192";
+ reg = <0>;
+ spi-max-frequency = <1000000>;
+ spi-cpol;
+ spi-cpha;
+ clocks = <&ad7192_mclk>;
+ clock-names = "mclk";
+ #interrupt-cells = <2>;
+ interrupts = <25 0x2>;
+ interrupt-parent = <&gpio>;
+ dvdd-supply = <&dvdd>;
+ avdd-supply = <&avdd>;
+
+ adi,refin2-pins-enable;
+ adi,rejection-60-Hz-enable;
+ adi,buffer-enable;
+ adi,burnout-currents-enable;
+ };
+ }
--
2.17.1
^ permalink raw reply related
* Re: [PATCH v3 4/4] ASoC: codecs: add wsa881x amplifier support
From: Srinivas Kandagatla @ 2019-08-12 9:05 UTC (permalink / raw)
To: kbuild test robot
Cc: kbuild-all, vkoul, broonie, bgoswami, plai, pierre-louis.bossart,
robh+dt, devicetree, lgirdwood, alsa-devel, linux-kernel
In-Reply-To: <201908121031.HBxXaLjU%lkp@intel.com>
Thanks for reporting this,
On 12/08/2019 03:46, kbuild test robot wrote:
> Hi Srinivas,
>
> I love your patch! Yet something to improve:
>
> [auto build test ERROR on linus/master]
> [cannot apply to v5.3-rc4 next-20190809]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>
> url: https://github.com/0day-ci/linux/commits/Srinivas-Kandagatla/ASoC-codecs-Add-WSA881x-Smart-Speaker-amplifier-support/20190812-080612
> config: m68k-allmodconfig (attached as .config)
> compiler: m68k-linux-gcc (GCC) 7.4.0
> reproduce:
> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # save the attached .config to linux build tree
> GCC_VERSION=7.4.0 make.cross ARCH=m68k
>
> If you fix the issue, kindly add following tag
> Reported-by: kbuild test robot <lkp@intel.com>
>
> All errors (new ones prefixed by >>):
>
>>> ERROR: "sdw_unregister_driver" [sound/soc/codecs/snd-soc-wsa881x.ko] undefined!
>>> ERROR: "sdw_write" [sound/soc/codecs/snd-soc-wsa881x.ko] undefined!
>>> ERROR: "__sdw_register_driver" [sound/soc/codecs/snd-soc-wsa881x.ko] undefined!
>>> ERROR: "sdw_write" [drivers/base/regmap/regmap-sdw.ko] undefined!
>>> ERROR: "sdw_read" [drivers/base/regmap/regmap-sdw.ko] undefined!
>
There are changes in SoundWire Kconfigs
(https://git.kernel.org/pub/scm/linux/kernel/git/vkoul/soundwire.git/commit/?h=fixes&id=8676b3ca4673517650fd509d7fa586aff87b3c28)
which are not available in linux/master yet so this error!
Once this patch lands then below errors should disappear.
thanks,
srini
> ---
> 0-DAY kernel test infrastructure Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all Intel Corporation
>
^ permalink raw reply
* Re: [PATCH 2/2] pwm: sprd: Add Spreadtrum PWM support
From: Baolin Wang @ 2019-08-12 9:11 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Thierry Reding, Rob Herring, Mark Rutland, Orson Zhai,
Chunyan Zhang, Vincent Guittot, linux-pwm, DTML, LKML, kernel
In-Reply-To: <20190812083556.dvprpwv6mjy3cjae@pengutronix.de>
Hi Uwe,
On Mon, 12 Aug 2019 at 16:36, Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
>
> Hello Baolin,
>
> On Mon, Aug 12, 2019 at 03:29:07PM +0800, Baolin Wang wrote:
> > Hi Uwe,
> >
> > On Fri, 9 Aug 2019 at 22:41, Uwe Kleine-König
> > <u.kleine-koenig@pengutronix.de> wrote:
> > > On Fri, Aug 09, 2019 at 06:06:21PM +0800, Baolin Wang wrote:
> > > > On Fri, 9 Aug 2019 at 17:10, Uwe Kleine-König
> > > > <u.kleine-koenig@pengutronix.de> wrote:
> > > > > On Thu, Aug 08, 2019 at 04:59:39PM +0800, Baolin Wang wrote:
> > > > > > +static void sprd_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
> > > > > > + struct pwm_state *state)
> > > > > > +{
> > > > > > + struct sprd_pwm_chip *spc =
> > > > > > + container_of(chip, struct sprd_pwm_chip, chip);
> > > > > > + struct sprd_pwm_chn *chn = &spc->chn[pwm->hwpwm];
> > > > > > + u32 enabled, duty, prescale;
> > > > > > + u64 tmp;
> > > > > > + int ret;
> > > > > > +
> > > > > > + ret = clk_bulk_prepare_enable(SPRD_PWM_NUM_CLKS, chn->clks);
> > > > > > + if (ret) {
> > > > > > + dev_err(spc->dev, "failed to enable pwm%u clocks\n",
> > > > > > + pwm->hwpwm);
> > > > > > + return;
> > > > > > + }
> > > > > > +
> > > > > > + chn->clk_enabled = true;
> > > > > > +
> > > > > > + duty = sprd_pwm_read(spc, pwm->hwpwm, SPRD_PWM_DUTY) & SPRD_PWM_REG_MSK;
> > > > > > + prescale = sprd_pwm_read(spc, pwm->hwpwm, SPRD_PWM_PRESCALE) & SPRD_PWM_REG_MSK;
> > > > > > + enabled = sprd_pwm_read(spc, pwm->hwpwm, SPRD_PWM_ENABLE) & SPRD_PWM_ENABLE_BIT;
> > > > > > +
> > > > > > + /*
> > > > > > + * According to the datasheet, the period_ns and duty_ns calculation
> > > > > > + * formula should be:
> > > > > > + * period_ns = 10^9 * (prescale + 1) * mod / clk_rate
> > > > > > + * duty_ns = 10^9 * (prescale + 1) * duty / clk_rate
> > > > > > + */
> > > > > > + tmp = (prescale + 1) * 1000000000ULL * SPRD_PWM_MOD_MAX;
> > > > > > + state->period = div64_u64(tmp, chn->clk_rate);
> > > > >
> > > > > This is not idempotent. If you apply the configuration that is returned
> > > > > here this shouldn't result in a reconfiguration.
> > > >
> > > > Since we may configure the PWM in bootloader, so in kernel part we
> > > > should get current PWM state to avoid reconfiguration if state
> > > > configuration are same.
> > >
> > > This is also important as some consumers might do something like:
> > >
> > > state = pwm_get_state(mypwm)
> > > if (something):
> > > state->duty = 0
> > > else:
> > > state->duty = state->period / 2
> > > pwm_set_state(mypwm, state)
> > >
> > > and then period shouldn't get smaller in each step.
> > > (This won't happen as of now because the PWM framework caches the last
> > > state that was set and returns this for pwm_get_state. Still getting
> > > this right would be good.)
> >
> > I understood your concern, but the period can be configured in
> > bootloader, we have no software things to save the accurate period.
>
> I don't understand what you're saying here. The bootloader configuring
> the hardware is a usual use-case. That's why we have the .get_state
> callback in the first place.
Ah, sorry for confusing. I think I get your point now with below explanation.
>
> > Moreover I think I can change to use DIV_ROUND_CLOSET_ULL to keep the
> > accuracy.
>
> DIV_ROUND_CLOSEST_ULL still doesn't match what the apply callback uses.
> With the lack of an official statement from the maintainer I'd prefer
> .apply to round down and implement .get_state such that
>
> pwm_apply(pwm, pwm_get_state(pwm))
>
> is a no-op.
OK.
>
> > > > > > +
> > > > > > + dev_err(spc->dev, "failed to get channel clocks\n");
> > > > > > + return ret;
> > > > > > + }
> > > > > > +
> > > > > > + clk_pwm = chn->clks[1].clk;
> > > > >
> > > > > This 1 looks suspicious. Are you using all clocks provided in the dtb at
> > > > > all? You're not using i in the loop at all, this doesn't look right.
> > > >
> > > > Like I said above, each channel has 2 clocks: enable clock and pwm
> > > > clock, the 2nd clock of each channel's bulk clocks is the pwm clock,
> > > > which is used to set the source clock. I know this's not easy to read,
> > > > so do you have any good suggestion?
> > >
> > > Not sure this is easily possible to rework to make this clearer.
> > >
> > > Do these clks have different uses? e.g. one to enable register access
> > > and the other to enable the pwm output? If so just using
> >
> > Yes.
>
> So assuming one of the clocks is for operation of the output and the
> other for accessing the registers, the latter can be disabled at the end
Right.
> of each callback?
We can not disable the latter one when using the PWM channel, we must
enable the pwm-enable clock, then enable pwm-output clock to make PWM
work. When disabling PWM channel, we should disable the pwm-output
clock, then pwm-enable clock.
>
> > > devm_clk_bulk_get isn't the right thing because you should be able know
> > > if clks[0] or clks[1] is the one you need to enable the output (or
> > > register access).
> >
> > We've fixed the clock order in bulk clocks by the array
> > 'sprd_pwm_clks', maybe I should define one readable macro instead of
> > magic number.
>
> ack.
>
> > > > > > + if (!clk_set_parent(clk_pwm, clk_parent))
> > > > > > + chn->clk_rate = clk_get_rate(clk_pwm);
> > > > > > + else
> > > > > > + chn->clk_rate = SPRD_PWM_DEFAULT_CLK;
> > > > >
> > > > > I don't know all the clock framework details, but I think there are
> > > > > better ways to ensure that a given clock is used as parent for another
> > > > > given clock. Please read the chapter about "Assigned clock parents and
> > > > > rates" in the clock bindings and check if this could be used for the
> > > > > purpose here and so simplify the driver.
> > > >
> > > > Actually there are many other drivers set the parent clock like this,
> > > > and we want a default clock if failed to set the parent clock.
> > >
> > > These might be older than the clk framework capabilities, or the
> > > reviewers didn't pay attention to this detail; both shouldn't be a
> > > reason to not make it better here.
> >
> > The clock framework supplies 'assigned-clocks' and
> > 'assigned-clock-parents' properties to set parent, but for our case we
> > still want to set a default clock rate if failed to set parent when
> > met some abnormal things.
>
> Without understanding the complete problem I'd say this is out of the
> area the driver should care about.
Fair enough, I will try to use 'assigned-clocks' and
'assigned-clock-parents' to simplify the code.
Thanks.
--
Baolin Wang
Best Regards
^ permalink raw reply
* RE: [PATCH v9 5/6] usb:cdns3 Add Cadence USB3 DRD Driver
From: Pawel Laszczak @ 2019-08-12 9:13 UTC (permalink / raw)
To: Felipe Balbi, devicetree@vger.kernel.org
Cc: gregkh@linuxfoundation.org, linux-usb@vger.kernel.org,
hdegoede@redhat.com, heikki.krogerus@linux.intel.com,
robh+dt@kernel.org, rogerq@ti.com, linux-kernel@vger.kernel.org,
jbergsagel@ti.com, nsekhar@ti.com, nm@ti.com, Suresh Punnoose,
peter.chen@nxp.com, Jayshri Dajiram Pawar, Rahul Kumar
In-Reply-To: <87imr2u77c.fsf@gmail.com>
Hi,
>
>Pawel Laszczak <pawell@cadence.com> writes:
>>>> I have such situation in which one interrupt line is shared with ehci and cdns3 driver.
>>>> In such case this function returns error code.
>>>
>>>which function returns error code?
>>
>> devm_request_threaded_irq, of course if I set IRQF_SHARED | IRQF_ONESHOT.
>> As I remember it was EBUSY error.
>
>oh, right. That's probably because the handlers must agree on IRQ flags.
>
>>>> So probably I will need to mask only the reported interrupts.
>>>
>>>you should mask all interrupts from your device, otherwise you top-halt
>>>may still end up reentrant.
>>>
>>>> I can't mask all interrupt using controller register because I can miss some of them.
>>>
>>>why would you miss them? They would be left in the register until you
>>>unmask them and the line is raised again.
>>
>> I consult this with author of controller.
>> We have:
>> USB_IEN and USB_ISTS for generic interrupts
>> EP_IEN and EP_ISTS for endpoint interrupts
>>
>> Both these group works different.
>> For endpoint I can disable all interrupt and I don't miss any of them.
>> So it's normal behavior.
>>
>> But USB_ISTS work little different. If we mask all interrupt in USB_IEN
>> then when new event occurs the EP_ISTS will not be updated.
>
>wait a minute. When you mask USB_ISTS, then EP_ISTS isn't updated? Is
>this a quirk on the controller or a design choice?
>
>> It's not standard and not expected behavior but it works in this way.
>
>Yeah, sounds rather odd.
>
Oh no. My mistake. Of course I mean USB_ISTS.
If we mask all interrupt in USB_IEN
then when new event occurs the USB_ISTS will not be updated.
>>>>>>>> + /* check USB device interrupt */
>>>>>>>> + reg = readl(&priv_dev->regs->usb_ists);
>>>>>>>> +
>>>>>>>> + if (reg) {
>>>>>>>> + writel(reg, &priv_dev->regs->usb_ists);
>>>>>>>> + cdns3_check_usb_interrupt_proceed(priv_dev, reg);
>>>>>>>> + ret = IRQ_HANDLED;
>>>>>>>
>>>>>>>now, because you _don't_ mask this interrupt, you're gonna have
>>>>>>>issues. Say we actually get both device and endpoint interrupts while
>>>>>>>the thread is already running with previous endpoint interrupts. Now
>>>>>>>we're gonna reenter the top half, because device interrupts are *not*
>>>>>>>masked, which will read usb_ists and handle it here.
>>>>>>
>>>>>> Endpoint interrupts are masked in cdns3_device_irq_handler and stay masked
>>>>>> until they are not handled in threaded handler.
>>>>>
>>>>>Quick question, then: these ISTS registers, are they masked interrupt
>>>>>status or raw interrupt status?
>>>>
>>>> Yes it's masked, but after masking them the new interrupts will not be reported
>>>> In ISTS registers. Form this reason I can mask only reported interrupt.
>>>
>>>and what happens when you unmask the registers? Do they get reported?
>>
>> No they are not reported in case of USB_ISTS register.
>> They should be reported in case EP_ISTS, but I need to test it.
>
>okay, please _do_ test and verify the behavior. The description above
>sounds really surprising to me. Does it really mean that if you mask all
>USB_ISTS and then disconnect the cable while interrupt is masked, you
>won't know cable was disconnected?
Yes, exactly.
Initially I've tested it and it's work correct.
I can even simply write 0 to EP_IEN in hard irq and ~0 in thread handler.
It's simplest and sufficient way.
>
>>>>>>>> + struct cdns3_aligned_buf *buf, *tmp;
>>>>>>>> +
>>>>>>>> + list_for_each_entry_safe(buf, tmp, &priv_dev->aligned_buf_list,
>>>>>>>> + list) {
>>>>>>>> + if (!buf->in_use) {
>>>>>>>> + list_del(&buf->list);
>>>>>>>> +
>>>>>>>> + spin_unlock_irqrestore(&priv_dev->lock, flags);
>>>>>>>
>>>>>>>creates the possibility of a race condition
>>>>>> Why? In this place the buf can't be used.
>>>>>
>>>>>but you're reenabling interrupts, right?
>>>>
>>>> Yes, driver frees not used buffers here.
>>>> I think that it's the safest place for this purpose.
>>>
>>>I guess you missed the point a little. Since you reenable interrupts
>>>just to free the buffer, you end up creating the possibility for a race
>>>condition. Specially since you don't mask all interrupt events. The
>>>moment you reenable interrupts, one of your not-unmasked interrupt
>>>sources could trigger, then top-half gets scheduled which tries to wake
>>>up the IRQ thread again and things go boom.
>>
>> Ok, I think I understand. So I have 3 options:
>> 1. Mask the USB_IEN and EP_IEN interrupts, but then I can lost some USB_ISTS
>> events. It's dangerous options.
>
>sure sounds dangerous, but also sounds quite "peculiar" :-)
>
>> 2. Remove implementation of handling unaligned buffers and assume that
>> upper layer will worry about this. What with vendor specific drivers that
>> can be used by companies and not upstreamed ?
>> It could be good to have such safety mechanism even if it is not currently used.
>
>dunno. It may become dead code that's NEVER used :-)
>
>> 3. Delegate this part of code for instance to separate thread that will be called
>> In free time.
>
>Yet another thread? Can't you just run this right before giving back the
>USB request? So, don't do it from IRQ handler, but from giveback path?
Do you mean in:
if (request->complete) {
spin_unlock(&priv_dev->lock);
if (priv_dev->run_garbage_collector) {
....
}
usb_gadget_giveback_request(&priv_ep->endpoint,
request);
spin_lock(&priv_dev->lock);
}
??
I ask because this is finally also called from IRQ handler:
cdns3_device_thread_irq_handler
-> cdns3_check_ep_interrupt_proceed
-> cdns3_transfer_completed
-> cdns3_gadget_giveback
-> usb_gadget_giveback_request
--
Pawell
^ permalink raw reply
* Re: [PATCH v3 05/21] ARM: dts: add recovery for I2C for iMX7
From: Michal Vokáč @ 2019-08-12 9:15 UTC (permalink / raw)
To: Philippe Schenker, Marcel Ziswiler, Max Krummenacher,
stefan@agner.ch, devicetree@vger.kernel.org, Rob Herring,
Shawn Guo, Mark Rutland, Fabio Estevam
Cc: Oleksandr Suvorov, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, Pengutronix Kernel Team,
NXP Linux Team, Sascha Hauer
In-Reply-To: <20190807082556.5013-6-philippe.schenker@toradex.com>
On 07. 08. 19 10:26, Philippe Schenker wrote:
> From: Oleksandr Suvorov <oleksandr.suvorov@toradex.com>
>
> - add recovery mode for applicable i2c buses for
> Colibri iMX7 module.
>
> Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com>
> Signed-off-by: Philippe Schenker <philippe.schenker@toradex.com>
> ---
Hi Philippe,
since you are going to send v4 anyway I suggest you update the subject
to be consistent across all the patches.
"ARM: dts: imx7-colibri: add recovery for I2C for iMX7"
fits better I think.
Thank you,
Michal
>
> Changes in v3: None
> Changes in v2: None
>
> arch/arm/boot/dts/imx7-colibri.dtsi | 25 +++++++++++++++++++++++--
> 1 file changed, 23 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/boot/dts/imx7-colibri.dtsi b/arch/arm/boot/dts/imx7-colibri.dtsi
> index a8d992f3e897..2480623c92ff 100644
> --- a/arch/arm/boot/dts/imx7-colibri.dtsi
> +++ b/arch/arm/boot/dts/imx7-colibri.dtsi
> @@ -140,8 +140,12 @@
>
> &i2c1 {
> clock-frequency = <100000>;
> - pinctrl-names = "default";
> + pinctrl-names = "default", "gpio";
> pinctrl-0 = <&pinctrl_i2c1 &pinctrl_i2c1_int>;
> + pinctrl-1 = <&pinctrl_i2c1_recovery &pinctrl_i2c1_int>;
> + scl-gpios = <&gpio1 4 GPIO_ACTIVE_HIGH>;
> + sda-gpios = <&gpio1 5 GPIO_ACTIVE_HIGH>;
> +
> status = "okay";
>
> codec: sgtl5000@a {
> @@ -242,8 +246,11 @@
>
> &i2c4 {
> clock-frequency = <100000>;
> - pinctrl-names = "default";
> + pinctrl-names = "default", "gpio";
> pinctrl-0 = <&pinctrl_i2c4>;
> + pinctrl-1 = <&pinctrl_i2c4_recovery>;
> + scl-gpios = <&gpio7 8 GPIO_ACTIVE_HIGH>;
> + sda-gpios = <&gpio7 9 GPIO_ACTIVE_HIGH>;
> };
>
> &lcdif {
> @@ -540,6 +547,13 @@
> >;
> };
>
> + pinctrl_i2c4_recovery: i2c4-recoverygrp {
> + fsl,pins = <
> + MX7D_PAD_ENET1_RGMII_TD2__GPIO7_IO8 0x4000007f
> + MX7D_PAD_ENET1_RGMII_TD3__GPIO7_IO9 0x4000007f
> + >;
> + };
> +
> pinctrl_lcdif_dat: lcdif-dat-grp {
> fsl,pins = <
> MX7D_PAD_LCD_DATA00__LCD_DATA0 0x79
> @@ -740,6 +754,13 @@
> >;
> };
>
> + pinctrl_i2c1_recovery: i2c1-recoverygrp {
> + fsl,pins = <
> + MX7D_PAD_LPSR_GPIO1_IO04__GPIO1_IO4 0x4000007f
> + MX7D_PAD_LPSR_GPIO1_IO05__GPIO1_IO5 0x4000007f
> + >;
> + };
> +
> pinctrl_cd_usdhc1: usdhc1-cd-grp {
> fsl,pins = <
> MX7D_PAD_LPSR_GPIO1_IO00__GPIO1_IO0 0x59 /* CD */
>
^ permalink raw reply
* Re: [PATCH 03/11] clocksource: sun4i: Add missing compatibles
From: Maxime Ripard @ 2019-08-12 9:16 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Mark Rutland, devicetree, Chen-Yu Tsai, Rob Herring, tglx,
Frank Rowand, linux-arm-kernel
In-Reply-To: <9df53981-d1b2-433c-e61f-7c000c71bc55@linaro.org>
[-- Attachment #1.1: Type: text/plain, Size: 570 bytes --]
Hi,
On Mon, Aug 12, 2019 at 10:59:51AM +0200, Daniel Lezcano wrote:
> On 22/07/2019 10:12, Maxime Ripard wrote:
> > Newer Allwinner SoCs have different number of interrupts, let's add
> > different compatibles for all of them to deal with this properly.
> >
> > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
>
> Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Thanks!
Can you merge this through your tree (along with the bindings)? I'll
merge the DT patches
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
[-- Attachment #2: 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 v8 1/4] arm64: dts: allwinner: orange-pi-3: Enable ethernet
From: Chen-Yu Tsai @ 2019-08-12 9:17 UTC (permalink / raw)
To: Ondřej Jirman
Cc: linux-sunxi, Maxime Ripard, Rob Herring, Jernej Škrabec,
David Airlie, Daniel Vetter, Mark Rutland, dri-devel, devicetree,
linux-arm-kernel, linux-kernel
In-Reply-To: <20190806155744.10263-2-megous-5qf/QAjKc83QT0dZR+AlfA@public.gmane.org>
On Tue, Aug 6, 2019 at 11:57 PM <megous-5qf/QAjKc83QT0dZR+AlfA@public.gmane.org> wrote:
>
> From: Ondrej Jirman <megous-5qf/QAjKc83QT0dZR+AlfA@public.gmane.org>
>
> Orange Pi 3 has two regulators that power the Realtek RTL8211E. According
> to the phy datasheet, both regulators need to be enabled at the same time,
> but we can only specify a single phy-supply in the DT.
>
> This can be achieved by making one regulator depedning on the other via
> vin-supply. While it's not a technically correct description of the
> hardware, it achieves the purpose.
>
> All values of RX/TX delay were tested exhaustively and a middle one of the
> working values was chosen.
>
> Signed-off-by: Ondrej Jirman <megous-5qf/QAjKc83QT0dZR+AlfA@public.gmane.org>
> ---
> .../dts/allwinner/sun50i-h6-orangepi-3.dts | 44 +++++++++++++++++++
> 1 file changed, 44 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts
> index 17d496990108..2c6807b74ff6 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts
> @@ -15,6 +15,7 @@
>
> aliases {
> serial0 = &uart0;
> + ethernet0 = &emac;
> };
>
> chosen {
> @@ -44,6 +45,27 @@
> regulator-max-microvolt = <5000000>;
> regulator-always-on;
> };
> +
> + /*
> + * The board uses 2.5V RGMII signalling. Power sequence to enable
> + * the phy is to enable GMAC-2V5 and GMAC-3V (aldo2) power rails
> + * at the same time and to wait 100ms.
> + */
> + reg_gmac_2v5: gmac-2v5 {
> + compatible = "regulator-fixed";
> + regulator-name = "gmac-2v5";
> + regulator-min-microvolt = <2500000>;
> + regulator-max-microvolt = <2500000>;
> + startup-delay-us = <100000>;
> + enable-active-high;
> + gpio = <&pio 3 6 GPIO_ACTIVE_HIGH>; /* PD6 */
> +
> + /* The real parent of gmac-2v5 is reg_vcc5v, but we need to
> + * enable two regulators to power the phy. This is one way
> + * to achieve that.
> + */
> + vin-supply = <®_aldo2>; /* GMAC-3V */
> + };
The RTL8211E datasheet I have says:
2.5V (or 1.8/1.5V) RGMII power should be risen simultaneously or slightly
earlier than 3.3V power. Rising 2.5V (or 1.8/1.5V) power later than 3.3V
power may lead to errors.
Since you can't reverse the parent relationship in your patch, maybe it's
time to add a phy-io-supply property?
It also says the rise time for 3.3V must be between 1ms and 100ms. However
the PMIC doesn't support voltage ramp control for the LDOs, nor does it list
the ramp rate.
ChenYu
> };
>
> &cpu0 {
> @@ -58,6 +80,28 @@
> status = "okay";
> };
>
> +&emac {
> + pinctrl-names = "default";
> + pinctrl-0 = <&ext_rgmii_pins>;
> + phy-mode = "rgmii";
> + phy-handle = <&ext_rgmii_phy>;
> + phy-supply = <®_gmac_2v5>;
> + allwinner,rx-delay-ps = <1500>;
> + allwinner,tx-delay-ps = <700>;
> + status = "okay";
> +};
> +
> +&mdio {
> + ext_rgmii_phy: ethernet-phy@1 {
> + compatible = "ethernet-phy-ieee802.3-c22";
> + reg = <1>;
> +
> + reset-gpios = <&pio 3 14 GPIO_ACTIVE_LOW>; /* PD14 */
> + reset-assert-us = <15000>;
> + reset-deassert-us = <40000>;
> + };
> +};
> +
> &mmc0 {
> vmmc-supply = <®_cldo1>;
> cd-gpios = <&pio 5 6 GPIO_ACTIVE_LOW>; /* PF6 */
> --
> 2.22.0
>
> --
> 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/20190806155744.10263-2-megous%40megous.com.
^ permalink raw reply
* Re: [PATCH v8 01/21] pinctrl: tegra: Fix write barrier placement in pmx_writel
From: Thierry Reding @ 2019-08-12 9:20 UTC (permalink / raw)
To: Sowjanya Komatineni
Cc: jonathanh, tglx, jason, marc.zyngier, linus.walleij, stefan,
mark.rutland, pdeschrijver, pgaikwad, sboyd, linux-clk,
linux-gpio, jckuo, josephl, talho, linux-tegra, linux-kernel,
mperttunen, spatra, robh+dt, digetx, devicetree, rjw,
viresh.kumar, linux-pm
In-Reply-To: <1565308020-31952-2-git-send-email-skomatineni@nvidia.com>
[-- Attachment #1: Type: text/plain, Size: 573 bytes --]
On Thu, Aug 08, 2019 at 04:46:40PM -0700, Sowjanya Komatineni wrote:
> pmx_writel uses writel which inserts write barrier before the
> register write rather.
>
> This patch has fix to replace writel with writel_relaxed followed
> by a write barrier to ensure write operation before the barrier
> is completed for successful pinctrl change.
>
> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
> ---
> drivers/pinctrl/tegra/pinctrl-tegra.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
Acked-by: Thierry Reding <treding@nvidia.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v8 02/21] pinctrl: tegra: Add write barrier after all pinctrl register writes
From: Thierry Reding @ 2019-08-12 9:20 UTC (permalink / raw)
To: Sowjanya Komatineni
Cc: jonathanh, tglx, jason, marc.zyngier, linus.walleij, stefan,
mark.rutland, pdeschrijver, pgaikwad, sboyd, linux-clk,
linux-gpio, jckuo, josephl, talho, linux-tegra, linux-kernel,
mperttunen, spatra, robh+dt, digetx, devicetree, rjw,
viresh.kumar, linux-pm
In-Reply-To: <1565308020-31952-3-git-send-email-skomatineni@nvidia.com>
[-- Attachment #1: Type: text/plain, Size: 409 bytes --]
On Thu, Aug 08, 2019 at 04:46:41PM -0700, Sowjanya Komatineni wrote:
> This patch adds write barrier after all pinctrl register writes
> during resume to make sure all pinctrl changes are complete.
>
> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
> ---
> drivers/pinctrl/tegra/pinctrl-tegra.c | 2 ++
> 1 file changed, 2 insertions(+)
Acked-by: Thierry Reding <treding@nvidia.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v8 03/21] clk: tegra: divider: Save and restore divider rate
From: Thierry Reding @ 2019-08-12 9:21 UTC (permalink / raw)
To: Sowjanya Komatineni
Cc: jonathanh, tglx, jason, marc.zyngier, linus.walleij, stefan,
mark.rutland, pdeschrijver, pgaikwad, sboyd, linux-clk,
linux-gpio, jckuo, josephl, talho, linux-tegra, linux-kernel,
mperttunen, spatra, robh+dt, digetx, devicetree, rjw,
viresh.kumar, linux-pm
In-Reply-To: <1565308020-31952-4-git-send-email-skomatineni@nvidia.com>
[-- Attachment #1: Type: text/plain, Size: 586 bytes --]
On Thu, Aug 08, 2019 at 04:46:42PM -0700, Sowjanya Komatineni wrote:
> This patch implements context restore for clock divider.
>
> During system suspend, core power goes off and looses the settings
> of the Tegra CAR controller registers.
>
> So on resume, clock dividers are restored back for normal operation.
>
> Reviewed-by: Dmitry Osipenko <digetx@gmail.com>
> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
> ---
> drivers/clk/tegra/clk-divider.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
Acked-by: Thierry Reding <treding@nvidia.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 03/11] clocksource: sun4i: Add missing compatibles
From: Daniel Lezcano @ 2019-08-12 9:21 UTC (permalink / raw)
To: Maxime Ripard
Cc: Mark Rutland, devicetree, Chen-Yu Tsai, Rob Herring, tglx,
Frank Rowand, linux-arm-kernel
In-Reply-To: <20190812091631.j2pr7i2zeput3hrc@flea>
[-- Attachment #1.1.1: Type: text/plain, Size: 824 bytes --]
On 12/08/2019 11:16, Maxime Ripard wrote:
> Hi,
>
> On Mon, Aug 12, 2019 at 10:59:51AM +0200, Daniel Lezcano wrote:
>> On 22/07/2019 10:12, Maxime Ripard wrote:
>>> Newer Allwinner SoCs have different number of interrupts, let's add
>>> different compatibles for all of them to deal with this properly.
>>>
>>> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
>>
>> Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>
> Thanks!
>
> Can you merge this through your tree (along with the bindings)? I'll
> merge the DT patches
patches 1-4 then ?
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: 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] ARM64: dts: allwinner: Add devicetree for pine H64 modelA evaluation board
From: Maxime Ripard @ 2019-08-12 9:40 UTC (permalink / raw)
To: Corentin Labbe
Cc: mark.rutland, robh+dt, wens, devicetree, linux-arm-kernel,
linux-kernel, linux-sunxi
In-Reply-To: <20190808084253.10573-1-clabbe.montjoie@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2369 bytes --]
On Thu, Aug 08, 2019 at 10:42:53AM +0200, Corentin Labbe wrote:
> This patch adds the evaluation variant of the model A of the PineH64.
> The model A has the same size of the pine64 and has a PCIE slot.
>
> The only devicetree difference with current pineH64, is the PHY
> regulator.
>
> Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
> ---
> arch/arm64/boot/dts/allwinner/Makefile | 1 +
> .../sun50i-h6-pine-h64-modelA-eval.dts | 26 +++++++++++++++++++
> 2 files changed, 27 insertions(+)
> create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64-modelA-eval.dts
>
> diff --git a/arch/arm64/boot/dts/allwinner/Makefile b/arch/arm64/boot/dts/allwinner/Makefile
> index f6db0611cb85..9a02166cbf72 100644
> --- a/arch/arm64/boot/dts/allwinner/Makefile
> +++ b/arch/arm64/boot/dts/allwinner/Makefile
> @@ -25,3 +25,4 @@ dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-orangepi-3.dtb
> dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-orangepi-lite2.dtb
> dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-orangepi-one-plus.dtb
> dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-pine-h64.dtb
> +dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-pine-h64-modelA-eval.dtb
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64-modelA-eval.dts b/arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64-modelA-eval.dts
> new file mode 100644
> index 000000000000..d8ff02747efe
> --- /dev/null
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64-modelA-eval.dts
> @@ -0,0 +1,26 @@
> +// SPDX-License-Identifier: (GPL-2.0+ or MIT)
> +/*
> + * Copyright (C) 2019 Corentin Labbe <clabbe.montjoie@gmail.com>
> + */
> +
> +#include "sun50i-h6-pine-h64.dts"
> +
> +/ {
> + model = "Pine H64 model A evaluation board";
> + compatible = "pine64,pine-h64-modelA-eval", "allwinner,sun50i-h6";
> +
> + reg_gmac_3v3: gmac-3v3 {
> + compatible = "regulator-fixed";
> + regulator-name = "vcc-gmac-3v3";
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + startup-delay-us = <100000>;
> + gpio = <&pio 2 16 GPIO_ACTIVE_HIGH>;
> + enable-active-high;
> + };
> +
> +};
> +
> +&emac {
> + phy-supply = <®_gmac_3v3>;
> +};
I might be missing some context here, but I'm pretty sure that the
initial intent of the pine h64 DTS was to support the model A all
along.
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
* RE: [v4 2/6] media: platform: dwc: Add MIPI CSI-2 controller driver
From: Luis de Oliveira @ 2019-08-12 9:45 UTC (permalink / raw)
To: Andy Shevchenko, Sakari Ailus
Cc: Luis Oliveira, Mauro Carvalho Chehab, David S. Miller,
Greg Kroah-Hartman, Jonathan Cameron, Rob Herring, Nicolas Ferre,
paulmck@linux.ibm.com, Mark Rutland, Kishon Vijay Abraham I,
devicetree, Linux Media Mailing List, Linux Kernel Mailing List,
Joao Pinto
In-Reply-To: <CAHp75VeutP=W43GHtY+FKvVGjBnQrF+nKbdaq_QXy8ZCoS=k1g@mail.gmail.com>
Hi Sakari, Andy,
From: Andy Shevchenko <andy.shevchenko@gmail.com>
Date: Sat, Aug 10, 2019 at 14:09:21
> On Fri, Aug 9, 2019 at 5:38 PM Sakari Ailus <sakari.ailus@iki.fi> wrote:
> > On Tue, Jun 11, 2019 at 09:20:51PM +0200, Luis Oliveira wrote:
> > > Add the Synopsys MIPI CSI-2 controller driver. This
> > > controller driver is divided in platform functions and core functions.
> > > This way it serves as platform for future DesignWare drivers.
>
> > > +const struct mipi_dt csi_dt[] = {
> >
> > Make this static or use a common prefix that somehow resembles the name
> > name of the driver.
I will do it.
> >
> > > + {
> > > + .hex = CSI_2_YUV420_8,
> > > + .name = "YUV420_8bits",
> > > + }, {
> > > + .hex = CSI_2_YUV420_10,
> > > + .name = "YUV420_10bits",
> > > + }, {
> > > + .hex = CSI_2_YUV420_8_LEG,
> > > + .name = "YUV420_8bits_LEGACY",
> > > + }, {
> > > + .hex = CSI_2_YUV420_8_SHIFT,
> > > + .name = "YUV420_8bits_SHIFT",
> > > + }, {
> > > + .hex = CSI_2_YUV420_10_SHIFT,
> > > + .name = "YUV420_10bits_SHIFT",
> > > + }, {
> > > + .hex = CSI_2_YUV422_8,
> > > + .name = "YUV442_8bits",
> > > + }, {
> > > + .hex = CSI_2_YUV422_10,
> > > + .name = "YUV442_10bits",
> > > + }, {
> > > + .hex = CSI_2_RGB444,
> > > + .name = "RGB444",
> > > + }, {
> > > + .hex = CSI_2_RGB555,
> > > + .name = "RGB555",
> > > + }, {
> > > + .hex = CSI_2_RGB565,
> > > + .name = "RGB565",
> > > + }, {
> > > + .hex = CSI_2_RGB666,
> > > + .name = "RGB666",
> > > + }, {
> > > + .hex = CSI_2_RGB888,
> > > + .name = "RGB888",
> > > + }, {
> > > + .hex = CSI_2_RAW6,
> > > + .name = "RAW6",
> > > + }, {
> > > + .hex = CSI_2_RAW7,
> > > + .name = "RAW7",
> > > + }, {
> > > + .hex = CSI_2_RAW8,
> > > + .name = "RAW8",
> > > + }, {
> > > + .hex = CSI_2_RAW10,
> > > + .name = "RAW10",
> > > + }, {
> > > + .hex = CSI_2_RAW12,
> > > + .name = "RAW12",
> > > + }, {
> > > + .hex = CSI_2_RAW14,
> > > + .name = "RAW14",
> > > + }, {
> > > + .hex = CSI_2_RAW16,
> > > + .name = "RAW16",
> > > + },
> > > +};
>
> One may utilize __stringify() macro and do somelike
>
> #define CSI_FMT_DESC(fmt) \
> { .hex = CSI_2_##fmt, .name = __stringify(fmt), }
>
> And do
>
> CSI_FMT_DESC(RAW16),
>
> etc.
>
Great, thanks!
> > > + return cfg ? v4l2_subdev_get_try_format(&dev->sd,
> > > + cfg,
> > > + 0) : NULL;
>
> This indentation looks ugly.
> I would rather put this on one line.
>
> > > + dev_dbg(dev->dev,
> > > + "%s got v4l2_mbus_pixelcode. 0x%x\n", __func__,
> > > + dev->format.code);
> > > + dev_dbg(dev->dev,
> > > + "%s got width. 0x%x\n", __func__,
> > > + dev->format.width);
> > > + dev_dbg(dev->dev,
> > > + "%s got height. 0x%x\n", __func__,
> > > + dev->format.height);
>
> __func__ is usually redundant (if Dynamic Debug in use it can be
> switched at run-time).
>
That's true, I don't need it.
> > I'd just omit these debug prints in a driver. But adding them to the
> > framework might make sense. We don't have a lot of debug prints dealing
> > with user parameters in there. OTOH the common test programs largely do the
> > same already.
>
> I would rather see tracepoints instead of debug prints if we are
> talking about generic solution for entire framework.
>
I will check that.
> >
> > > + return &dev->format;
> > > +}
>
> > > + struct mipi_fmt *dev_fmt;
>
> This is simple bad name. We have dev_fmt() macro. I would rather avoid
> potential collisions.
True, I will change the name.
>
> > > + struct v4l2_mbus_framefmt *mf;
> > > +
> > > + mf = dw_mipi_csi_get_format(dev, cfg, fmt->which);
> > > + if (!mf)
> > > + return -EINVAL;
>
> Can't you rather return an error pointer in this and similar cases?
>
Yes, ofc.
> > > + dev_vdbg(dev->dev, "%s: on=%d\n", __func__, on);
>
> This is noise. If you would like to debug Function Tracer is a good start.
>
Ok.
> > > + of_id = of_match_node(dw_mipi_csi_of_match, dev->of_node);
> > > + if (!of_id)
> > > + return -EINVAL;
>
> Is it possible to have this asserted?
>
I will remove it.
> > > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>
> > > + if (!res)
> > > + return -ENXIO;
>
> Redundant. Below does the check for you.
>
Yep, thanks.
> > > +
> > > + csi->base_address = devm_ioremap_resource(dev, res);
> > > + if (IS_ERR(csi->base_address)) {
>
> > > + dev_err(dev, "Base address not set.\n");
>
> Redundant. Above does print an error message for you.
>
Ok.
> > > + return PTR_ERR(csi->base_address);
> > > + }
>
> Moreover, use devm_platform_ioremap_resource() instead of both.
>
Nice, thanks.
> > > + csi->ctrl_irq_number = platform_get_irq(pdev, 0);
> > > + if (csi->ctrl_irq_number < 0) {
>
> > > + dev_err(dev, "irq number %d not set.\n", csi->ctrl_irq_number);
>
> Redundant since this cycle (v5.4).
>
Ok,
> > > + ret = csi->ctrl_irq_number;
>
> Better to do the opposite
>
> ret = platform_get_irq();
> if (ret)
> goto end;
> ... = ret;
>
> > > + goto end;
> > > + }
>
> > > + ret = devm_request_irq(dev, csi->ctrl_irq_number,
> > > + dw_mipi_csi_irq1, IRQF_SHARED,
> > > + dev_name(dev), csi);
> > > + if (ret) {
> > > + dev_err(dev, "irq csi %s failed\n", of_id->name);
> > > +
> > > + goto end;
> > > + }
>
> devm_*irq() might be a bad idea. Is it race free in your driver?
>
I never thought about it like that. Should I use request_irq and
free_irq?
> > > +static const struct of_device_id dw_mipi_csi_of_match[] = {
> > > + { .compatible = "snps,dw-csi" },
>
> > > + {},
>
> Better without comma. Terminator may terminate even at compile time.
>
Ok.
> > > +};
>
> > > +static ssize_t core_version_show(struct device *dev,
> > > + struct device_attribute *attr,
> > > + char *buf)
> > > +{
> > > + struct platform_device *pdev = to_platform_device(dev);
> > > + struct v4l2_subdev *sd = platform_get_drvdata(pdev);
> > > + struct dw_csi *csi_dev = sd_to_mipi_csi_dev(sd);
>
> > > +
> > > + char buffer[10];
> > > +
> > > + snprintf(buffer, 10, "v.%d.%d*\n", csi_dev->hw_version_major,
> > > + csi_dev->hw_version_minor);
> > > +
> > > + return strlcpy(buf, buffer, PAGE_SIZE);
>
> Oh, can't you simple without any temprorary useless buffers?
> sprintf(buf, ...)?
> (Yes, note _absence_ of *n* there)
You are right.
>
> > > +}
>
> > > +static ssize_t n_lanes_store(struct device *dev, struct device_attribute *attr,
> > > + const char *buf, size_t count)
> > > +{
> > > + int ret;
> > > + unsigned long lanes;
>
> > > +
>
> More blank lines! We need them!
>
Ok.
> > > + struct platform_device *pdev = to_platform_device(dev);
> > > + struct v4l2_subdev *sd = platform_get_drvdata(pdev);
> > > + struct dw_csi *csi_dev = sd_to_mipi_csi_dev(sd);
> > > +
> > > + ret = kstrtoul(buf, 10, &lanes);
> > > + if (ret < 0)
> > > + return ret;
>
> Can it return positive number?
>
> > > + dev_info(dev, "Lanes %lu\n", lanes);
>
> Noise.
> The user gets it, why to spam kernel log???
>
Ok.
> > > + csi_dev->hw.num_lanes = lanes;
> > > +
> > > + return count;
> > > +}
>
> I told once, can repeat again. Synopsys perhaps needs better reviews
> inside company. Each time I see the code, it repeats same mistakes
> over and over. Have you, guys, do something about it?
We are working on it. It will get better, sorry.
>
> --
> With Best Regards,
> Andy Shevchenko
Thanks,
Luis
^ permalink raw reply
* RE: [PATCH v9 5/6] usb:cdns3 Add Cadence USB3 DRD Driver
From: Felipe Balbi @ 2019-08-12 9:45 UTC (permalink / raw)
To: Pawel Laszczak, devicetree@vger.kernel.org
Cc: gregkh@linuxfoundation.org, linux-usb@vger.kernel.org,
hdegoede@redhat.com, heikki.krogerus@linux.intel.com,
robh+dt@kernel.org, rogerq@ti.com, linux-kernel@vger.kernel.org,
jbergsagel@ti.com, nsekhar@ti.com, nm@ti.com, Suresh Punnoose,
peter.chen@nxp.com, Jayshri Dajiram Pawar, Rahul Kumar
In-Reply-To: <BYAPR07MB4709C07ED94C952886858F14DDD30@BYAPR07MB4709.namprd07.prod.outlook.com>
[-- Attachment #1: Type: text/plain, Size: 3750 bytes --]
Hi,
Pawel Laszczak <pawell@cadence.com> writes:
>>>>>>Quick question, then: these ISTS registers, are they masked interrupt
>>>>>>status or raw interrupt status?
>>>>>
>>>>> Yes it's masked, but after masking them the new interrupts will not be reported
>>>>> In ISTS registers. Form this reason I can mask only reported interrupt.
>>>>
>>>>and what happens when you unmask the registers? Do they get reported?
>>>
>>> No they are not reported in case of USB_ISTS register.
>>> They should be reported in case EP_ISTS, but I need to test it.
>>
>>okay, please _do_ test and verify the behavior. The description above
>>sounds really surprising to me. Does it really mean that if you mask all
>>USB_ISTS and then disconnect the cable while interrupt is masked, you
>>won't know cable was disconnected?
>
> Yes, exactly.
>
> Initially I've tested it and it's work correct.
> I can even simply write 0 to EP_IEN in hard irq and ~0 in thread handler.
> It's simplest and sufficient way.
okay. Just to be sure I understand correctly. If you mask USB_IEN, then
we would miss a cable disconnect event. Right?
>>>>>>>>> + struct cdns3_aligned_buf *buf, *tmp;
>>>>>>>>> +
>>>>>>>>> + list_for_each_entry_safe(buf, tmp, &priv_dev->aligned_buf_list,
>>>>>>>>> + list) {
>>>>>>>>> + if (!buf->in_use) {
>>>>>>>>> + list_del(&buf->list);
>>>>>>>>> +
>>>>>>>>> + spin_unlock_irqrestore(&priv_dev->lock, flags);
>>>>>>>>
>>>>>>>>creates the possibility of a race condition
>>>>>>> Why? In this place the buf can't be used.
>>>>>>
>>>>>>but you're reenabling interrupts, right?
>>>>>
>>>>> Yes, driver frees not used buffers here.
>>>>> I think that it's the safest place for this purpose.
>>>>
>>>>I guess you missed the point a little. Since you reenable interrupts
>>>>just to free the buffer, you end up creating the possibility for a race
>>>>condition. Specially since you don't mask all interrupt events. The
>>>>moment you reenable interrupts, one of your not-unmasked interrupt
>>>>sources could trigger, then top-half gets scheduled which tries to wake
>>>>up the IRQ thread again and things go boom.
>>>
>>> Ok, I think I understand. So I have 3 options:
>>> 1. Mask the USB_IEN and EP_IEN interrupts, but then I can lost some USB_ISTS
>>> events. It's dangerous options.
>>
>>sure sounds dangerous, but also sounds quite "peculiar" :-)
>>
>>> 2. Remove implementation of handling unaligned buffers and assume that
>>> upper layer will worry about this. What with vendor specific drivers that
>>> can be used by companies and not upstreamed ?
>>> It could be good to have such safety mechanism even if it is not currently used.
>>
>>dunno. It may become dead code that's NEVER used :-)
>>
>>> 3. Delegate this part of code for instance to separate thread that will be called
>>> In free time.
>>
>>Yet another thread? Can't you just run this right before giving back the
>>USB request? So, don't do it from IRQ handler, but from giveback path?
>
> Do you mean in:
> if (request->complete) {
> spin_unlock(&priv_dev->lock);
> if (priv_dev->run_garbage_collector) {
> ....
> }
> usb_gadget_giveback_request(&priv_ep->endpoint,
> request);
> spin_lock(&priv_dev->lock);
> }
> ??
right, you can do it right before giving back the request. Or right
after.
> I ask because this is finally also called from IRQ handler:
>
> cdns3_device_thread_irq_handler
> -> cdns3_check_ep_interrupt_proceed
> -> cdns3_transfer_completed
> -> cdns3_gadget_giveback
> -> usb_gadget_giveback_request
Did you notice that it doesn't reenable interrupts, though?
--
balbi
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply
* Re: [PATCH 2/2] pwm: sprd: Add Spreadtrum PWM support
From: Uwe Kleine-König @ 2019-08-12 9:46 UTC (permalink / raw)
To: Baolin Wang
Cc: Thierry Reding, Rob Herring, Mark Rutland, Orson Zhai,
Chunyan Zhang, Vincent Guittot, linux-pwm, DTML, LKML, kernel
In-Reply-To: <CAMz4kuJA81ZP6Kc63dPV1jEn1ah=jow6tQBfO=UDCcTzSf3y-A@mail.gmail.com>
Hello Baolin,
On Mon, Aug 12, 2019 at 05:11:56PM +0800, Baolin Wang wrote:
> On Mon, 12 Aug 2019 at 16:36, Uwe Kleine-König
> <u.kleine-koenig@pengutronix.de> wrote:
> > On Mon, Aug 12, 2019 at 03:29:07PM +0800, Baolin Wang wrote:
> > > The clock framework supplies 'assigned-clocks' and
> > > 'assigned-clock-parents' properties to set parent, but for our case we
> > > still want to set a default clock rate if failed to set parent when
> > > met some abnormal things.
> >
> > Without understanding the complete problem I'd say this is out of the
> > area the driver should care about.
>
> Fair enough, I will try to use 'assigned-clocks' and
> 'assigned-clock-parents' to simplify the code.
There is also assigned-clock-rates if you need that.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* Re: [PATCH v8 07/21] clk: Add API to get index of the clock parent
From: Thierry Reding @ 2019-08-12 9:47 UTC (permalink / raw)
To: Sowjanya Komatineni
Cc: jonathanh, tglx, jason, marc.zyngier, linus.walleij, stefan,
mark.rutland, pdeschrijver, pgaikwad, sboyd, linux-clk,
linux-gpio, jckuo, josephl, talho, linux-tegra, linux-kernel,
mperttunen, spatra, robh+dt, digetx, devicetree, rjw,
viresh.kumar, linux-pm
In-Reply-To: <1565308020-31952-8-git-send-email-skomatineni@nvidia.com>
[-- Attachment #1: Type: text/plain, Size: 1400 bytes --]
On Thu, Aug 08, 2019 at 04:46:46PM -0700, Sowjanya Komatineni wrote:
> This patch adds an API clk_hw_get_parent_index to get index of the
> clock parent to use during the clock restore operations on system
> resume.
>
> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
> ---
> drivers/clk/clk.c | 17 +++++++++++++++++
> include/linux/clk-provider.h | 1 +
> 2 files changed, 18 insertions(+)
>
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index c0990703ce54..f26252e48f73 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -1643,6 +1643,23 @@ static int clk_fetch_parent_index(struct clk_core *core,
> return i;
> }
>
> +/**
> + * clk_hw_get_parent_index - return the index of parent clock
> + * @hw: clk_hw associated with the clk being consumed
> + * @parent_hw: clk_hw associated with the parent of clk
> + *
> + * Fetches and returns the index of parent clock.
> + * if hw or parent_hw is NULL, returns -EINVAL.
"If" because it's at the beginning of a sentence. You may also want to
turn this into a "Return:" section as described in:
Documentation/doc-guide/kernel-doc.rst
That said, other functions in this file don't use that construct either,
so I suppose this is fine as-is, for consistency.
So with the capitalization of "If" fixed, this is:
Reviewed-by: Thierry Reding <treding@nvidia.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ 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