* Re: [PATCH 00/12] usb: dt-bindings: samsung: convert to dtschema (subset)
From: Krzysztof Kozlowski @ 2022-01-25 17:02 UTC (permalink / raw)
To: linux-samsung-soc, Greg Kroah-Hartman, linux-usb,
Krzysztof Kozlowski, linux-kernel, linux-arm-kernel, devicetree,
Rob Herring
In-Reply-To: <20220123111644.25540-1-krzysztof.kozlowski@canonical.com>
On Sun, 23 Jan 2022 12:16:32 +0100, Krzysztof Kozlowski wrote:
> Dependencies
> ============
> None.
>
> The DTS patches are independent and I will take them via Samsung SoC tree.
> I am including them here just so automatic robot checks won't complain about
> DTS differences against newly dtschema.
>
> [...]
Applied subset, thanks!
[01/12] arm64: dts: exynos: add USB DWC3 supplies to Espresso board
commit: 31c33503fdb3965d6aaf0db4a8c42e7d8cef1dff
[02/12] ARM: dts: exynos: add USB DWC3 supplies to Arndale
commit: 52d53d937da8889964c60216a0333cb19fe0812d
[03/12] ARM: dts: exynos: add USB DWC3 supplies to SMDK5250
commit: ebbb07b8d349fc2eccb67780850d2d1bbfc918d6
[04/12] ARM: dts: exynos: add USB DWC3 supplies to Chromebook Snow
commit: 9745be7b5a3be39a00e6bbda3305e2d789ee4082
[05/12] ARM: dts: exynos: add USB DWC3 supplies to Chromebook Spring
commit: 111ea2d6dd217684db4e7a97a2bda3bf14734427
[06/12] ARM: dts: exynos: add USB DWC3 supplies to ArndaleOcta
commit: 0a14272479627bb9388ece3b0ebac72a3928062d
[07/12] ARM: dts: exynos: add USB DWC3 supplies to Chromebook Peach Pit
commit: 7adf978462dadc41ea7d4138de53bc9a15922191
[08/12] ARM: dts: exynos: add USB DWC3 supplies to Chromebook Peach Pi
commit: 72477416ac12e88384a96575c5f2e4bd7ac8feeb
[09/12] ARM: dts: exynos: add USB DWC3 supplies to SMDK5420
commit: c441d2d73107fcb45c0affb345fe6b9bc3fd3bab
[10/12] ARM: dts: exynos: add fake USB DWC3 supplies to SMDK5410
commit: 4043114504cc05d0a7ca2a061838699b500599cd
Best regards,
--
Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
^ permalink raw reply
* Re: [PATCH 0/2] ARM: dts: exynos: Add support for Samsung Klimt WiFi
From: Krzysztof Kozlowski @ 2022-01-25 17:02 UTC (permalink / raw)
To: robh+dt, linux-arm-kernel, Henrik Grimler, virag.david003,
m.szyprowski, cw00.choi, ~postmarketos/upstreaming, alim.akhtar,
linux-kernel, linux-samsung-soc, devicetree, semen.protsenko
Cc: Krzysztof Kozlowski
In-Reply-To: <20220124131241.29946-1-henrik@grimler.se>
On Mon, 24 Jan 2022 14:12:39 +0100, Henrik Grimler wrote:
> Klimt WiFi has product name Samsung Galaxy Tab S 8.4" and was released
> in 2014. It has similar hardware to exynos5420-chagall-wifi and the
> two devices can hence share a common base dtsi.
>
> Same issues as for Chagall Wifi applies: CCI has to be disabled and
> only 4 out of 8 CPUs are brought up at boot (see
> https://lore.kernel.org/r/20220118185746.299832-4-henrik@grimler.se).
>
> [...]
Applied, thanks!
[1/2] dt-bindings: arm: samsung: document Klimt WiFi board binding
commit: 8d4d0d7c43e6f87df02a0acac206b50c265962d1
[2/2] ARM: dts: exynos: Add support for Samsung Klimt WiFi
commit: 21fc732222559c7b5a8731571d30501d5fcb631c
Best regards,
--
Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
^ permalink raw reply
* Re: [PATCH v3 0/7] MT8186 SMI SUPPORT
From: Krzysztof Kozlowski @ 2022-01-25 16:55 UTC (permalink / raw)
To: Yong Wu, Matthias Brugger, Rob Herring
Cc: Krzysztof Kozlowski, anan.sun, youlin.pei, srv_heupstream,
Tomasz Figa, devicetree, Joerg Roedel, iommu, linux-mediatek,
AngeloGioacchino Del Regno, yi.kuo, linux-arm-kernel,
linux-kernel, lc.kan, anthony.huang
In-Reply-To: <20220113111057.29918-1-yong.wu@mediatek.com>
On Thu, 13 Jan 2022 19:10:50 +0800, Yong Wu wrote:
> This patchset adds mt8186 smi support.
> mainly adds a sleep control function.
>
> Change note:
> v3: a) Add a new binding patch for renaming "clock" to "clocks".
> b) Reword the title for the binding patches, more detailed.
> c) Add the sleep control error path: if err, return directly.
> also change the log from dev_warn to dev_err.
>
> [...]
Applied, thanks!
[1/7] dt-bindings: memory: mtk-smi: Rename clock to clocks
commit: 5bf7fa48374eafe29dbb30448a0b0c083853583f
[2/7] dt-bindings: memory: mtk-smi: No need mediatek,larb-id for mt8167
commit: ddc3a324889686ec9b358de20fdeec0d2668c7a8
[3/7] dt-bindings: memory: mtk-smi: Correct minItems to 2 for the gals clocks
commit: 996ebc0e332bfb3091395f9bd286d8349a57be62
[4/7] dt-bindings: memory: mediatek: Add mt8186 support
commit: 6d86f23c35fe7b479ceef4d3f1eef925996945fd
[5/7] memory: mtk-smi: Fix the return value for clk_bulk_prepare_enable
commit: a6945f4566d4f77a4054720f6649ff921fe1ae64
[6/7] memory: mtk-smi: Add sleep ctrl function
commit: 8956500e5d5bf541a945299999b0bf4866dc0daf
[7/7] memory: mtk-smi: mt8186: Add smi support
commit: 86a010bfc73983aa8cd914f1e5f73962b0406678
Best regards,
--
Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
^ permalink raw reply
* Re: [PATCH V3 10/10] arm64: dts: imx8mm: Enable Hantro G1 and G2 video decoders
From: Ezequiel Garcia @ 2022-01-25 16:48 UTC (permalink / raw)
To: Adam Ford
Cc: linux-media, linux-arm-kernel, shawnguo, aford, Philipp Zabel,
Mauro Carvalho Chehab, Rob Herring, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
Greg Kroah-Hartman, Lucas Stach, linux-rockchip, devicetree,
linux-kernel, linux-staging
In-Reply-To: <20220124023125.414794-11-aford173@gmail.com>
Hi Adam,
On Sun, Jan 23, 2022 at 08:31:24PM -0600, Adam Ford wrote:
> There are two decoders on the i.MX8M Mini controlled by the
> vpu-blk-ctrl. The G1 supports H264 and VP8 while the
> G2 support HEVC and VP9.
>
> Signed-off-by: Adam Ford <aford173@gmail.com>
>
Looks good.
Reviewed-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
Thanks,
Ezequiel
> diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> index 0c7a72c51a31..98aec4421713 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> @@ -1272,6 +1272,22 @@ gpu_2d: gpu@38008000 {
> power-domains = <&pgc_gpu>;
> };
>
> + vpu_g1: video-codec@38300000 {
> + compatible = "nxp,imx8mm-vpu-g1";
> + reg = <0x38300000 0x10000>;
> + interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&clk IMX8MM_CLK_VPU_G1_ROOT>;
> + power-domains = <&vpu_blk_ctrl IMX8MM_VPUBLK_PD_G1>;
> + };
> +
> + vpu_g2: video-codec@38310000 {
> + compatible = "nxp,imx8mq-vpu-g2";
> + reg = <0x38310000 0x10000>;
> + interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&clk IMX8MM_CLK_VPU_G2_ROOT>;
> + power-domains = <&vpu_blk_ctrl IMX8MM_VPUBLK_PD_G2>;
> + };
> +
> vpu_blk_ctrl: blk-ctrl@38330000 {
> compatible = "fsl,imx8mm-vpu-blk-ctrl", "syscon";
> reg = <0x38330000 0x100>;
> @@ -1282,6 +1298,12 @@ vpu_blk_ctrl: blk-ctrl@38330000 {
> <&clk IMX8MM_CLK_VPU_G2_ROOT>,
> <&clk IMX8MM_CLK_VPU_H1_ROOT>;
> clock-names = "g1", "g2", "h1";
> + assigned-clocks = <&clk IMX8MM_CLK_VPU_G1>,
> + <&clk IMX8MM_CLK_VPU_G2>;
> + assigned-clock-parents = <&clk IMX8MM_VPU_PLL_OUT>,
> + <&clk IMX8MM_VPU_PLL_OUT>;
> + assigned-clock-rates = <600000000>,
> + <600000000>;
> #power-domain-cells = <1>;
> };
>
> --
> 2.32.0
>
^ permalink raw reply
* Re: [PATCH] drm/mediatek: DP HPD Detect with debounce
From: Chun-Kuang Hu @ 2022-01-25 16:44 UTC (permalink / raw)
To: Jitao Shi
Cc: Chun-Kuang Hu, Philipp Zabel, Matthias Brugger, Daniel Vetter,
David Airlie, DRI Development, linux-kernel,
moderated list:ARM/Mediatek SoC support, Linux ARM, CK Hu,
stonea168, huijuan.xie, Rex-BC Chen, shuijing.li, Rob Herring,
DTML, Guillaume Ranquet
In-Reply-To: <20220112150133.11275-1-jitao.shi@mediatek.com>
Hi, Jitao:
Jitao Shi <jitao.shi@mediatek.com> 於 2022年1月12日 週三 下午11:01寫道：
>
> DP Spec 1.4a 3.3 requires dp detect the hpd with debounce.
>
> Upstream implementations should implement HPD signal de-bouncing on
> an external connection. A period of 100ms should be used for
> detecting an HPD connect event (i.e., the event, “HPD high,” is
> confirmed only after HPD has been continuously asserted for 100ms).
> Care should be taken to not implement de-bouncing on an IRQ_HPD and
> on a downstream device-generated pair of HPD disconnect/reconnect
> events (typically HPD shall be de-asserted for more than 2ms, but
> less than 100ms in this case). To cover these cases, HPD de-bounce
> should be implemented only after HPD low has been detected for 100ms.
> Timing requirements in this Standard related to the detection of
> HPD high are to be interpreted as applying from the completion of an
> implementation-dependent de-bounce period.
>
> Signed-off-by: Jitao Shi <jitao.shi@mediatek.com>
> ---
mtk dp driver has not been upstreamed yet. This patch seems depend on
another series [1]. Please describe the dependency information here.
[1] https://patchwork.kernel.org/project/linux-mediatek/list/?series=597485
Regards,
Chun-Kuang.
> drivers/gpu/drm/mediatek/mtk_dp.c | 24 ++++++++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c b/drivers/gpu/drm/mediatek/mtk_dp.c
> index a256d55346a2..05f401a024a4 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dp.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dp.c
> @@ -193,6 +193,8 @@ struct mtk_dp {
> struct mutex eld_lock;
> u8 connector_eld[MAX_ELD_BYTES];
> struct drm_connector *conn;
> + bool need_debounce;
> + struct timer_list debounce_timer;
> };
>
> enum mtk_dp_sdp_type {
> @@ -2217,6 +2219,9 @@ static irqreturn_t mtk_dp_hpd_event_thread(int hpd, void *dev)
> if (event < 0)
> return IRQ_HANDLED;
>
> + if (mtk_dp->need_debounce && mtk_dp->train_info.cable_plugged_in)
> + msleep(100);
> +
> if (mtk_dp->drm_dev) {
> dev_info(mtk_dp->dev, "drm_helper_hpd_irq_event\n");
> drm_helper_hpd_irq_event(mtk_dp->bridge.dev);
> @@ -2296,6 +2301,14 @@ static irqreturn_t mtk_dp_hpd_isr_handler(struct mtk_dp *mtk_dp)
> mtk_dp->train_state = MTK_DP_TRAIN_STATE_STARTUP;
> }
> train_info->cable_state_change = true;
> +
> + if (train_info->cable_state_change) {
> + if (!train_info->cable_plugged_in) {
> + mod_timer(&mtk_dp->debounce_timer, jiffies + msecs_to_jiffies(100) - 1);
> + mtk_dp->need_debounce = false;
> + }
> + }
> +
> return IRQ_WAKE_THREAD;
> }
>
> @@ -2903,6 +2916,13 @@ static int mtk_dp_register_audio_driver(struct device *dev)
> return 0;
> }
>
> +static void mtk_dp_debounce_timer(struct timer_list *t)
> +{
> + struct mtk_dp *mtk_dp = from_timer(mtk_dp, t, debounce_timer);
> +
> + mtk_dp->need_debounce = true;
> +}
> +
> static int mtk_dp_probe(struct platform_device *pdev)
> {
> struct mtk_dp *mtk_dp;
> @@ -2990,6 +3010,9 @@ static int mtk_dp_probe(struct platform_device *pdev)
> else
> mtk_dp->bridge.type = DRM_MODE_CONNECTOR_DisplayPort;
>
> + mtk_dp->need_debounce = true;
> + timer_setup(&mtk_dp->debounce_timer, mtk_dp_debounce_timer, 0);
> +
> mtk_dp->bridge.ops =
> DRM_BRIDGE_OP_DETECT | DRM_BRIDGE_OP_EDID | DRM_BRIDGE_OP_HPD;
> drm_bridge_add(&mtk_dp->bridge);
> @@ -3008,6 +3031,7 @@ static int mtk_dp_remove(struct platform_device *pdev)
>
> mtk_dp_video_mute(mtk_dp, true);
> mtk_dp_audio_mute(mtk_dp, true);
> + del_timer_sync(&mtk_dp->debounce_timer);
>
> pm_runtime_disable(&pdev->dev);
>
> --
> 2.12.5
>
^ permalink raw reply
* Re: [PATCH v10 1/3] pwm: driver for qualcomm ipq6018 pwm block
From: Baruch Siach @ 2022-01-25 16:22 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Thierry Reding, Andy Gross, Bjorn Andersson, Balaji Prakash J,
Rob Herring, Robert Marko, Kathiravan T, linux-pwm, devicetree,
linux-arm-msm, linux-arm-kernel
In-Reply-To: <20220125161204.hx5foivny6iupjke@pengutronix.de>
Hi Uwe,
On Tue, Jan 25 2022, Uwe Kleine-König wrote:
> On Tue, Jan 25, 2022 at 03:03:08PM +0200, Baruch Siach wrote:
>> On Wed, Jan 19 2022, Uwe Kleine-König wrote:
>> > The task here is to calculate the biggest pwm_div for a given pre_div
>> > such that
>> >
>> >
>> > (pre_div + 1) * (pwm_div + 1) * NSEC_PER_SEC
>> > -------------------------------------------- <= period_ns
>> > rate
>> >
>> > right?
>> >
>> > This is equivalent to:
>> >
>> > period_ns * rate
>> > pre_div <= ---------------------------- - 1
>> > (pre_div + 1) * NSEC_PER_SEC
>> >
>> > As pre_div is integer, rounding down should be fine?!
>>
>> I can't follow. With round down (as in v8) the result is always:
>>
>> NSEC_PER_SEC * (pre_div + 1) * (pwm_div + 1) <= period_rate
>
> Yes, that's the condition that a valid configuration should fulfill
> because then the configured period is never bigger than the requested
> period.
>
>> As a result, 'diff' calculation below will always produce diff <= 0. When
>> there is no diff == 0 result (bingo) we get IPQ_PWM_MAX_DIV in both best_
>> values at the end of the loop.
>
> Looking again, your check is wrong. I think you need:
>
> diff = period_rate - NSEC_PER_SEC * (pre_div + 1) * (pwm_div + 1)
>
> . Given the calculations for pre_div and pwm_div this should never be
> negative and you should pick values that minimize diff.
So, if I understand correctly, you suggest to leave round up as in v10,
and invert the diff calculation. Is that correct?
Thanks,
baruch
--
~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- baruch@tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -
^ permalink raw reply
* [PATCH 1/3] dt-bindings: usb: dwc2: add compatible "intel,socfpga-agilex-hsotg"
From: Dinh Nguyen @ 2022-01-25 16:18 UTC (permalink / raw)
To: hminas; +Cc: dinguyen, gregkh, robh+dt, linux-usb, devicetree
Add the compatible "intel,socfpga-agilex-hsotg" to the DWC2
implementation, because the Agilex DWC2 implementation does not support
clock gating.
Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
---
Documentation/devicetree/bindings/usb/dwc2.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/usb/dwc2.yaml b/Documentation/devicetree/bindings/usb/dwc2.yaml
index f00867ebc147..481aaa09f3f2 100644
--- a/Documentation/devicetree/bindings/usb/dwc2.yaml
+++ b/Documentation/devicetree/bindings/usb/dwc2.yaml
@@ -53,6 +53,7 @@ properties:
- const: st,stm32mp15-hsotg
- const: snps,dwc2
- const: samsung,s3c6400-hsotg
+ - const: intel,socfpga-agilex-hsotg
reg:
maxItems: 1
--
2.25.1
^ permalink raw reply related
* [PATCH 2/3] usb: dwc2: Add platform specific data for Intel's Agilex
From: Dinh Nguyen @ 2022-01-25 16:18 UTC (permalink / raw)
To: hminas; +Cc: dinguyen, gregkh, robh+dt, linux-usb, devicetree
In-Reply-To: <20220125161821.1951906-1-dinguyen@kernel.org>
The DWC2 IP on the Agilex platform does not support clock-gating.
Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
---
drivers/usb/dwc2/params.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index d300ae3d9274..1306f4ec788d 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -82,6 +82,14 @@ static void dwc2_set_s3c6400_params(struct dwc2_hsotg *hsotg)
p->phy_utmi_width = 8;
}
+static void dwc2_set_socfpga_agilex_params(struct dwc2_hsotg *hsotg)
+{
+ struct dwc2_core_params *p = &hsotg->params;
+
+ p->power_down = DWC2_POWER_DOWN_PARAM_NONE;
+ p->no_clock_gating = true;
+}
+
static void dwc2_set_rk_params(struct dwc2_hsotg *hsotg)
{
struct dwc2_core_params *p = &hsotg->params;
@@ -239,6 +247,8 @@ const struct of_device_id dwc2_of_match_table[] = {
.data = dwc2_set_stm32mp15_fsotg_params },
{ .compatible = "st,stm32mp15-hsotg",
.data = dwc2_set_stm32mp15_hsotg_params },
+ { .compatible = "intel,socfpga-agilex-hsotg",
+ .data = dwc2_set_socfpga_agilex_params },
{},
};
MODULE_DEVICE_TABLE(of, dwc2_of_match_table);
--
2.25.1
^ permalink raw reply related
* [PATCH 3/3] arm64: dts: agilex: use the compatible "intel,socfpga-agilex-hsotg"
From: Dinh Nguyen @ 2022-01-25 16:18 UTC (permalink / raw)
To: hminas; +Cc: dinguyen, gregkh, robh+dt, linux-usb, devicetree
In-Reply-To: <20220125161821.1951906-1-dinguyen@kernel.org>
The DWC2 USB controller on the Agilex platform does not support clock
gating, so use the chip specific "intel,socfpga-agilex-hsotg"
compatible.
Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
---
arch/arm64/boot/dts/intel/socfpga_agilex.dtsi | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/intel/socfpga_agilex.dtsi b/arch/arm64/boot/dts/intel/socfpga_agilex.dtsi
index 0dd2d2ee765a..f4270cf18996 100644
--- a/arch/arm64/boot/dts/intel/socfpga_agilex.dtsi
+++ b/arch/arm64/boot/dts/intel/socfpga_agilex.dtsi
@@ -502,7 +502,7 @@ uart1: serial@ffc02100 {
};
usb0: usb@ffb00000 {
- compatible = "snps,dwc2";
+ compatible = "intel,socfpga-agilex-hsotg", "snps,dwc2";
reg = <0xffb00000 0x40000>;
interrupts = <GIC_SPI 93 IRQ_TYPE_LEVEL_HIGH>;
phys = <&usbphy0>;
@@ -515,7 +515,7 @@ usb0: usb@ffb00000 {
};
usb1: usb@ffb40000 {
- compatible = "snps,dwc2";
+ compatible = "intel,socfpga-agilex-hsotg", "snps,dwc2";
reg = <0xffb40000 0x40000>;
interrupts = <GIC_SPI 94 IRQ_TYPE_LEVEL_HIGH>;
phys = <&usbphy0>;
--
2.25.1
^ permalink raw reply related
* Re: [PATCH 3/4] iio: adc: xilinx-ams: Fixed wrong sequencer register settings
From: Robert Hancock @ 2022-01-25 16:15 UTC (permalink / raw)
To: m.tretter@pengutronix.de
Cc: lars@metafoo.de, robh+dt@kernel.org, jic23@kernel.org,
devicetree@vger.kernel.org, michal.simek@xilinx.com,
linux-iio@vger.kernel.org, manish.narani@xilinx.com,
linux-arm-kernel@lists.infradead.org, kernel@pengutronix.de,
anand.ashok.dumbre@xilinx.com
In-Reply-To: <20220125082108.GE25856@pengutronix.de>
On Tue, 2022-01-25 at 09:21 +0100, Michael Tretter wrote:
> On Wed, 19 Jan 2022 19:02:45 -0600, Robert Hancock wrote:
> > Register settings used for the sequencer configuration register
> > were incorrect, causing some inputs to not be read properly.
> >
> > Fixes: d5c70627a794 ("iio: adc: Add Xilinx AMS driver")
> > Signed-off-by: Robert Hancock <robert.hancock@calian.com>
> > ---
> > drivers/iio/adc/xilinx-ams.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/iio/adc/xilinx-ams.c b/drivers/iio/adc/xilinx-ams.c
> > index b93864362dac..199027c93cdc 100644
> > --- a/drivers/iio/adc/xilinx-ams.c
> > +++ b/drivers/iio/adc/xilinx-ams.c
> > @@ -91,8 +91,8 @@
> >
> > #define AMS_CONF1_SEQ_MASK GENMASK(15, 12)
> > #define AMS_CONF1_SEQ_DEFAULT FIELD_PREP(AMS_CONF1_SEQ_MASK,
> > 0)
> > -#define AMS_CONF1_SEQ_CONTINUOUS FIELD_PREP(AMS_CONF1_SEQ_MASK, 1)
> > -#define AMS_CONF1_SEQ_SINGLE_CHANNEL FIELD_PREP(AMS_CONF1_SEQ_MASK,
> > 2)
> > +#define AMS_CONF1_SEQ_CONTINUOUS FIELD_PREP(AMS_CONF1_SEQ_MASK, 2)
> > +#define AMS_CONF1_SEQ_SINGLE_CHANNEL FIELD_PREP(AMS_CONF1_SEQ_MASK,
> > 3)
>
> The TRM states that Continuous Loop Mode is 2, but Single Pass Sequence Mode
> is 1, not 3. Is there a reason, why you need to set both bits?
Single pass sequence mode (1) just runs the same sequence only once. To read
these values it needs to switch to single channel mode (3).
The register bits are defined in Table 3-8 of
https://www.xilinx.com/support/documentation/user_guides/ug580-ultrascale-sysmon.pdf
.
>
> Michael
>
> >
> > #define AMS_REG_SEQ0_MASK GENMASK(15, 0)
> > #define AMS_REG_SEQ2_MASK GENMASK(21, 16)
> > --
> > 2.31.1
> >
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > https://urldefense.com/v3/__http://lists.infradead.org/mailman/listinfo/linux-arm-kernel__;!!IOGos0k!yGFEjSC1BL20lwurby914len0HCLXyzarwxKJP9Jx30qv_qrERSkRJUiVo_2MdusMVA$
> >
--
Robert Hancock
Senior Hardware Designer, Calian Advanced Technologies
www.calian.com
^ permalink raw reply
* Re: [PATCH v10 1/3] pwm: driver for qualcomm ipq6018 pwm block
From: Uwe Kleine-König @ 2022-01-25 16:12 UTC (permalink / raw)
To: Baruch Siach
Cc: Thierry Reding, Andy Gross, Bjorn Andersson, Balaji Prakash J,
Rob Herring, Robert Marko, Kathiravan T, linux-pwm, devicetree,
linux-arm-msm, linux-arm-kernel
In-Reply-To: <87tuds7y09.fsf@tarshish>
[-- Attachment #1: Type: text/plain, Size: 15671 bytes --]
Hello Baruch,
On Tue, Jan 25, 2022 at 03:03:08PM +0200, Baruch Siach wrote:
> On Wed, Jan 19 2022, Uwe Kleine-König wrote:
> > On Tue, Dec 14, 2021 at 06:27:17PM +0200, Baruch Siach wrote:
> >> From: Baruch Siach <baruch.siach@siklu.com>
> >>
> >> Driver for the PWM block in Qualcomm IPQ6018 line of SoCs. Based on
> >> driver from downstream Codeaurora kernel tree. Removed support for older
> >> (V1) variants because I have no access to that hardware.
> >>
> >> Tested on IPQ6010 based hardware.
> >>
> >> Signed-off-by: Baruch Siach <baruch.siach@siklu.com>
> >> ---
> >> v10:
> >>
> >> Restore round up in pwm_div calculation; otherwise diff is always <=
> >> 0, so only bingo match works
> >>
> >> Don't overwrite min_diff on every loop iteration
> >>
> >> v9:
> >>
> >> Address comment from Uwe Kleine-König:
> >>
> >> Use period_ns*rate in dividers calculation for better accuracy
> >>
> >> Round down pre_div and pwm_div
> >>
> >> Add a comment explaining why pwm_div can't underflow
> >>
> >> Add a comment explaining why pre_div > pwm_div end the search loop
> >>
> >> Drop 'CFG_' from register macros
> >>
> >> Rename to_ipq_pwm_chip() to ipq_pwm_from_chip()
> >>
> >> Change bare 'unsigned' to 'unsigned int'
> >>
> >> Clarify the comment on separate REG1 write for enable/disable
> >>
> >> Round up the period value in .get_state
> >>
> >> Use direct readl/writel so no need to check for regmap errors
> >>
> >> v7:
> >>
> >> Change 'offset' to 'reg' for the tcsr offset (Rob)
> >>
> >> Drop clock name; there is only one clock (Bjorn)
> >>
> >> Simplify probe failure code path (Bjorn)
> >>
> >> v6:
> >>
> >> Address Uwe Kleine-König review comments:
> >>
> >> Drop IPQ_PWM_MAX_DEVICES
> >>
> >> Rely on assigned-clock-rates; drop IPQ_PWM_CLK_SRC_FREQ
> >>
> >> Simplify register offset calculation
> >>
> >> Calculate duty cycle more precisely
> >>
> >> Refuse to set inverted polarity
> >>
> >> Drop redundant IPQ_PWM_REG1_ENABLE bit clear
> >>
> >> Remove x1000 factor in pwm_div calculation, use rate directly, and round up
> >>
> >> Choose initial pre_div such that pwm_div < IPQ_PWM_MAX_DIV
> >>
> >> Ensure pre_div <= pwm_div
> >>
> >> Rename close_ to best_
> >>
> >> Explain in comment why effective_div doesn't overflow
> >>
> >> Limit pwm_div to IPQ_PWM_MAX_DIV - 1 to allow 100% duty cycle
> >>
> >> Disable clock only after pwmchip_remove()
> >>
> >> const pwm_ops
> >>
> >> Other changes:
> >>
> >> Add missing linux/bitfield.h header include (kernel test robot)
> >>
> >> Adjust code for PWM device node under TCSR (Rob Herring)
> >>
> >> v5:
> >>
> >> Use &tcsr_q6 syscon to access registers (Bjorn Andersson)
> >>
> >> Address Uwe Kleine-König review comments:
> >>
> >> Implement .get_state()
> >>
> >> Add IPQ_PWM_ prefix to local macros
> >>
> >> Use GENMASK/BIT/FIELD_PREP for register fields access
> >>
> >> Make type of config_div_and_duty() parameters consistent
> >>
> >> Derive IPQ_PWM_MIN_PERIOD_NS from IPQ_PWM_CLK_SRC_FREQ
> >>
> >> Integrate enable/disable into config_div_and_duty() to save register read,
> >> and reduce frequency glitch on update
> >>
> >> Use min() instead of min_t()
> >>
> >> Fix comment format
> >>
> >> Use dev_err_probe() to indicate probe step failure
> >>
> >> Add missing clk_disable_unprepare() in .remove
> >>
> >> Don't set .owner
> >>
> >> v4:
> >>
> >> Use div64_u64() to fix link for 32-bit targets ((kernel test robot
> >> <lkp@intel.com>, Uwe Kleine-König)
> >>
> >> v3:
> >>
> >> s/qcom,pwm-ipq6018/qcom,ipq6018-pwm/ (Rob Herring)
> >>
> >> Fix integer overflow on 32-bit targets (kernel test robot <lkp@intel.com>)
> >>
> >> v2:
> >>
> >> Address Uwe Kleine-König review comments:
> >>
> >> Fix period calculation when out of range
> >>
> >> Don't set period larger than requested
> >>
> >> Remove PWM disable on configuration change
> >>
> >> Implement .apply instead of non-atomic .config/.enable/.disable
> >>
> >> Don't modify PWM on .request/.free
> >>
> >> Check pwm_div underflow
> >>
> >> Fix various code and comment formatting issues
> >>
> >> Other changes:
> >>
> >> Use u64 divisor safe division
> >>
> >> Remove now empty .request/.free
> >> ---
> >> drivers/pwm/Kconfig | 12 ++
> >> drivers/pwm/Makefile | 1 +
> >> drivers/pwm/pwm-ipq.c | 275 ++++++++++++++++++++++++++++++++++++++++++
> >> 3 files changed, 288 insertions(+)
> >> create mode 100644 drivers/pwm/pwm-ipq.c
> >>
> >> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
> >> index 21e3b05a5153..e39718137ecd 100644
> >> --- a/drivers/pwm/Kconfig
> >> +++ b/drivers/pwm/Kconfig
> >> @@ -260,6 +260,18 @@ config PWM_INTEL_LGM
> >> To compile this driver as a module, choose M here: the module
> >> will be called pwm-intel-lgm.
> >>
> >> +config PWM_IPQ
> >> + tristate "IPQ PWM support"
> >> + depends on ARCH_QCOM || COMPILE_TEST
> >> + depends on HAVE_CLK && HAS_IOMEM
> >> + help
> >> + Generic PWM framework driver for IPQ PWM block which supports
> >> + 4 pwm channels. Each of the these channels can be configured
> >> + independent of each other.
> >> +
> >> + To compile this driver as a module, choose M here: the module
> >> + will be called pwm-ipq.
> >> +
> >> config PWM_IQS620A
> >> tristate "Azoteq IQS620A PWM support"
> >> depends on MFD_IQS62X || COMPILE_TEST
> >> diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
> >> index 708840b7fba8..7402feae4b36 100644
> >> --- a/drivers/pwm/Makefile
> >> +++ b/drivers/pwm/Makefile
> >> @@ -22,6 +22,7 @@ obj-$(CONFIG_PWM_IMX1) += pwm-imx1.o
> >> obj-$(CONFIG_PWM_IMX27) += pwm-imx27.o
> >> obj-$(CONFIG_PWM_IMX_TPM) += pwm-imx-tpm.o
> >> obj-$(CONFIG_PWM_INTEL_LGM) += pwm-intel-lgm.o
> >> +obj-$(CONFIG_PWM_IPQ) += pwm-ipq.o
> >> obj-$(CONFIG_PWM_IQS620A) += pwm-iqs620a.o
> >> obj-$(CONFIG_PWM_JZ4740) += pwm-jz4740.o
> >> obj-$(CONFIG_PWM_KEEMBAY) += pwm-keembay.o
> >> diff --git a/drivers/pwm/pwm-ipq.c b/drivers/pwm/pwm-ipq.c
> >> new file mode 100644
> >> index 000000000000..3764010808f0
> >> --- /dev/null
> >> +++ b/drivers/pwm/pwm-ipq.c
> >> @@ -0,0 +1,275 @@
> >> +// SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0
> >> +/*
> >> + * Copyright (c) 2016-2017, 2020 The Linux Foundation. All rights reserved.
> >> + */
> >> +
> >> +#include <linux/module.h>
> >> +#include <linux/platform_device.h>
> >> +#include <linux/pwm.h>
> >> +#include <linux/clk.h>
> >> +#include <linux/io.h>
> >> +#include <linux/of.h>
> >> +#include <linux/math64.h>
> >> +#include <linux/of_device.h>
> >> +#include <linux/bitfield.h>
> >> +
> >> +/* The frequency range supported is 1 Hz to clock rate */
> >> +#define IPQ_PWM_MAX_PERIOD_NS ((u64)NSEC_PER_SEC)
> >> +
> >> +/*
> >> + * The max value specified for each field is based on the number of bits
> >> + * in the pwm control register for that field
> >> + */
> >> +#define IPQ_PWM_MAX_DIV 0xFFFF
> >> +
> >> +/*
> >> + * Two 32-bit registers for each PWM: REG0, and REG1.
> >> + * Base offset for PWM #i is at 8 * #i.
> >> + */
> >> +#define IPQ_PWM_REG0 0 /*PWM_DIV PWM_HI*/
> >> +#define IPQ_PWM_REG0_PWM_DIV GENMASK(15, 0)
> >> +#define IPQ_PWM_REG0_HI_DURATION GENMASK(31, 16)
> >
> > PWM_HI in the comment of IPQ_PWM_REG0 vs. HI_DURATION? Should this
> > match? I'd say the comment is redundant.
> >
> >> +#define IPQ_PWM_REG1 4 /*ENABLE UPDATE PWM_PRE_DIV*/
> >> +#define IPQ_PWM_REG1_PRE_DIV GENMASK(15, 0)
> >> +/*
> >> + * Enable bit is set to enable output toggling in pwm device.
> >> + * Update bit is set to reflect the changed divider and high duration
> >> + * values in register.
> >> + */
> >> +#define IPQ_PWM_REG1_UPDATE BIT(30)
> >> +#define IPQ_PWM_REG1_ENABLE BIT(31)
> >> +
> >> +
> >> +struct ipq_pwm_chip {
> >> + struct pwm_chip chip;
> >> + struct clk *clk;
> >> + void __iomem *mem;
> >> +};
> >> +
> >> +static struct ipq_pwm_chip *ipq_pwm_from_chip(struct pwm_chip *chip)
> >> +{
> >> + return container_of(chip, struct ipq_pwm_chip, chip);
> >> +}
> >> +
> >> +static unsigned int ipq_pwm_reg_read(struct pwm_device *pwm, unsigned int reg)
> >> +{
> >> + struct ipq_pwm_chip *ipq_chip = ipq_pwm_from_chip(pwm->chip);
> >> + unsigned int off = 8 * pwm->hwpwm + reg;
> >> +
> >> + return readl(ipq_chip->mem + off);
> >> +}
> >> +
> >> +static void ipq_pwm_reg_write(struct pwm_device *pwm, unsigned int reg,
> >> + unsigned int val)
> >> +{
> >> + struct ipq_pwm_chip *ipq_chip = ipq_pwm_from_chip(pwm->chip);
> >> + unsigned int off = 8 * pwm->hwpwm + reg;
> >> +
> >> + writel(val, ipq_chip->mem + off);
> >> +}
> >> +
> >> +static void config_div_and_duty(struct pwm_device *pwm, unsigned int pre_div,
> >> + unsigned int pwm_div, unsigned long rate, u64 duty_ns,
> >> + bool enable)
> >> +{
> >> + unsigned long hi_dur;
> >> + unsigned long val = 0;
> >> +
> >> + /*
> >> + * high duration = pwm duty * (pwm div + 1)
> >> + * pwm duty = duty_ns / period_ns
> >> + */
> >> + hi_dur = div64_u64(duty_ns * rate, (pre_div + 1) * NSEC_PER_SEC);
> >> +
> >> + val = FIELD_PREP(IPQ_PWM_REG0_HI_DURATION, hi_dur) |
> >> + FIELD_PREP(IPQ_PWM_REG0_PWM_DIV, pwm_div);
> >> + ipq_pwm_reg_write(pwm, IPQ_PWM_REG0, val);
> >> +
> >> + val = FIELD_PREP(IPQ_PWM_REG1_PRE_DIV, pre_div);
> >> + ipq_pwm_reg_write(pwm, IPQ_PWM_REG1, val);
> >> +
> >> + /* PWM enable toggle needs a separate write to REG1 */
> >> + val |= IPQ_PWM_REG1_UPDATE;
> >> + if (enable)
> >> + val |= IPQ_PWM_REG1_ENABLE;
> >> + ipq_pwm_reg_write(pwm, IPQ_PWM_REG1, val);
> >> +}
> >> +
> >> +static int ipq_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> >> + const struct pwm_state *state)
> >> +{
> >> + struct ipq_pwm_chip *ipq_chip = ipq_pwm_from_chip(chip);
> >> + unsigned int pre_div, pwm_div, best_pre_div, best_pwm_div;
> >> + unsigned long rate = clk_get_rate(ipq_chip->clk);
> >> + u64 period_ns, duty_ns, period_rate;
> >> + u64 min_diff;
> >> +
> >> + if (state->polarity != PWM_POLARITY_NORMAL)
> >> + return -EINVAL;
> >> +
> >> + if (state->period < div64_u64(NSEC_PER_SEC, rate))
> >> + return -ERANGE;
> >
> > NSEC_PER_SEC / rate is the smallest period you can achieve, right?
> > Consider rate = 33333 (Hz), then the minimal period is
> > 30000.30000300003 ns. So you should refuse a request to configure
> > state->period = 30000, but as div64_u64(1000000000, 33333) is 30000 you
> > don't.
> >
> >> + period_ns = min(state->period, IPQ_PWM_MAX_PERIOD_NS);
> >> + duty_ns = min(state->duty_cycle, period_ns);
> >> +
> >> + /*
> >> + * period_ns is 1G or less. As long as rate is less than 16 GHz this
> >> + * does not overflow.
> >
> > Well, rate cannot be bigger than 4294967295 because an unsigned long
> > cannot hold a bigger value.
>
> On 64-bit systems __SIZEOF_LONG__ is 8, which can hold more than 2^32.
Ah right, then I suggest to check that in code to make it more explicit
than in a comment.
> >> + */
> >> + period_rate = period_ns * rate;
> >> + best_pre_div = IPQ_PWM_MAX_DIV;
> >> + best_pwm_div = IPQ_PWM_MAX_DIV;
> >> + /* Initial pre_div value such that pwm_div < IPQ_PWM_MAX_DIV */
>
> Note this comment.
<= ?
>
> >> + pre_div = div64_u64(period_rate,
> >> + (u64)NSEC_PER_SEC * (IPQ_PWM_MAX_DIV + 1));
> >
> > Hmmm, we want
> >
> > (pre_div + 1) * (pwm_div + 1) * NSEC_PER_SEC
> > -------------------------------------------- <= period_ns
> > rate
> >
> > , right? Resolving that for pre_div this gives:
> >
> > period_ns * rate
> > pre_div <= ----------------------------
> > NSEC_PER_SEC * (pwm_div + 1)
> >
> > The term on the right hand side is maximal for pwm_div == 0 so the
> > possible values for pre_div are
> >
> > 0 ... min(div64_u64(period_rate / NSEC_PER_SEC), IPQ_PWM_MAX_DIV)
> >
> > isn't it?
>
> I don't think so. pre_div == 0 will produce pwm_div larger than
> IPQ_PWM_MAX_DIV for a large period_rate value. The initial pre_div here is the
> smallest value that produces pwm_div within it limit.
Ah, got your reasoning. If a pre_div is picked that is smaller than the
value you calculate, a bigger pre_div results in a better approximation.
What strikes me is that if you pick a smaller pre_div, you can still use
pwm_div = IPQ_PWM_MAX_DIV which yields an allowed setting. So while your
argument is right, I'd need a better comment to actually understand it.
Something like:
/*
* We don't need to consider pre_div values smaller than
*
* period_rate
* pre_div_min := ------------------------------------
* NSEC_PER_SEC * (IPQ_PWM_MAX_DIV + 1)
*
* because pre_div = pre_div_min results in a better
* approximation.
*/
> > If so, your algorithm is wrong as you're iterating over
> >
> > div64_u64(period_rate, NSEC_PER_SEC * (IPQ_PWM_MAX_DIV + 1)) ... IPQ_PWM_MAX_DIV
>
> The loop stops when pre_div > pwm_div. That should be before pre_div ==
> IPQ_PWM_MAX_DIV because pwm_div <= IPQ_PWM_MAX_DIV. Should I put the pre_div >
> pwm_div condition directly in the for statement?
OK, moving the check isn't necessary.
> > The task here is to calculate the biggest pwm_div for a given pre_div
> > such that
> >
> >
> > (pre_div + 1) * (pwm_div + 1) * NSEC_PER_SEC
> > -------------------------------------------- <= period_ns
> > rate
> >
> > right?
> >
> > This is equivalent to:
> >
> > period_ns * rate
> > pre_div <= ---------------------------- - 1
> > (pre_div + 1) * NSEC_PER_SEC
> >
> > As pre_div is integer, rounding down should be fine?!
>
> I can't follow. With round down (as in v8) the result is always:
>
> NSEC_PER_SEC * (pre_div + 1) * (pwm_div + 1) <= period_rate
Yes, that's the condition that a valid configuration should fulfill
because then the configured period is never bigger than the requested
period.
> As a result, 'diff' calculation below will always produce diff <= 0. When
> there is no diff == 0 result (bingo) we get IPQ_PWM_MAX_DIV in both best_
> values at the end of the loop.
Looking again, your check is wrong. I think you need:
diff = period_rate - NSEC_PER_SEC * (pre_div + 1) * (pwm_div + 1)
. Given the calculations for pre_div and pwm_div this should never be
negative and you should pick values that minimize diff.
> Do we actually need diff > 0 in the condition below?
>
> >> + /*
> >> + * pre_div and pwm_div values swap produces the same
> >> + * result. This loop goes over all pre_div <= pwm_div
> >> + * combinations. The rest are equivalent.
> >> + */
> >
> > I'd write:
> >
> > /*
> > * Swapping values for pre_div and pwm_div produces the same
> > * period length. So we can skip all settings with pre_div <
> > * pwm_div which results in bigger constraints for selecting the
> > * duty_cycle than with the two values swapped.
> > */
>
> I'll take your wording with inverted inequality sign.
Right.
Looking forward to your next iteration.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* [RFC PATCH v2 2/2] leds: Add PWM multicolor driver
From: sven @ 2022-01-25 15:12 UTC (permalink / raw)
To: linux-leds, devicetree, linux-pwm
Cc: Sven Schwermer, pavel, dmurphy, robh+dt, thierry.reding,
u.kleine-koenig, lee.jones, post
In-Reply-To: <20220125151226.31049-1-sven@svenschwermer.de>
From: Sven Schwermer <sven.schwermer@disruptive-technologies.com>
By allowing to group multiple monochrome PWM LEDs into multicolor LEDs,
all involved LEDs can be controlled in-sync. This enables using effects
using triggers, etc.
Signed-off-by: Sven Schwermer <sven.schwermer@disruptive-technologies.com>
---
drivers/leds/Kconfig | 8 ++
drivers/leds/Makefile | 1 +
drivers/leds/leds-pwm-multicolor.c | 184 +++++++++++++++++++++++++++++
3 files changed, 193 insertions(+)
create mode 100644 drivers/leds/leds-pwm-multicolor.c
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 6090e647daee..bae1f63f6195 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -552,6 +552,14 @@ config LEDS_PWM
help
This option enables support for pwm driven LEDs
+config LEDS_PWM_MULTICOLOR
+ tristate "PWM driven multi-color LED Support"
+ depends on LEDS_CLASS_MULTICOLOR
+ depends on PWM
+ help
+ This option enables support for PWM driven monochrome LEDs that are
+ grouped into multicolor LEDs.
+
config LEDS_REGULATOR
tristate "REGULATOR driven LED support"
depends on LEDS_CLASS
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index e58ecb36360f..ba2c2c1edf12 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -73,6 +73,7 @@ obj-$(CONFIG_LEDS_PCA963X) += leds-pca963x.o
obj-$(CONFIG_LEDS_PM8058) += leds-pm8058.o
obj-$(CONFIG_LEDS_POWERNV) += leds-powernv.o
obj-$(CONFIG_LEDS_PWM) += leds-pwm.o
+obj-$(CONFIG_LEDS_PWM_MULTICOLOR) += leds-pwm-multicolor.o
obj-$(CONFIG_LEDS_REGULATOR) += leds-regulator.o
obj-$(CONFIG_LEDS_S3C24XX) += leds-s3c24xx.o
obj-$(CONFIG_LEDS_SC27XX_BLTC) += leds-sc27xx-bltc.o
diff --git a/drivers/leds/leds-pwm-multicolor.c b/drivers/leds/leds-pwm-multicolor.c
new file mode 100644
index 000000000000..c54bed4536d3
--- /dev/null
+++ b/drivers/leds/leds-pwm-multicolor.c
@@ -0,0 +1,184 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * PWM-based multi-color LED control
+ *
+ * Copyright 2022 Sven Schwermer <sven.schwermer@disruptive-technologies.com>
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/platform_device.h>
+#include <linux/led-class-multicolor.h>
+#include <linux/leds.h>
+#include <linux/err.h>
+#include <linux/pwm.h>
+#include <linux/mutex.h>
+
+struct pwm_led {
+ struct pwm_device *pwm;
+ struct pwm_state pwmstate;
+};
+
+struct pwm_mc_led {
+ struct led_classdev_mc mc_cdev;
+ struct mutex lock;
+ struct pwm_led leds[];
+};
+
+static int led_pwm_mc_set(struct led_classdev *cdev,
+ enum led_brightness brightness)
+{
+ int i;
+ unsigned long long duty;
+ int ret = 0;
+ struct led_classdev_mc *mc_cdev = lcdev_to_mccdev(cdev);
+ struct pwm_mc_led *priv = container_of(mc_cdev, struct pwm_mc_led, mc_cdev);
+
+ led_mc_calc_color_components(mc_cdev, brightness);
+
+ mutex_lock(&priv->lock);
+
+ for (i = 0; i < mc_cdev->num_colors; ++i) {
+ duty = priv->leds[i].pwmstate.period;
+ duty *= mc_cdev->subled_info[i].brightness;
+ do_div(duty, cdev->max_brightness);
+
+ priv->leds[i].pwmstate.duty_cycle = duty;
+ priv->leds[i].pwmstate.enabled = duty > 0;
+ ret = pwm_apply_state(priv->leds[i].pwm,
+ &priv->leds[i].pwmstate);
+ if (ret)
+ break;
+ }
+
+ mutex_unlock(&priv->lock);
+
+ return ret;
+}
+
+static int led_pwm_mc_probe(struct platform_device *pdev)
+{
+ struct fwnode_handle *mcnode, *fwnode;
+ int count = 0;
+ struct pwm_mc_led *priv;
+ struct mc_subled *subled;
+ struct led_classdev *cdev;
+ struct pwm_led *pwmled;
+ u32 color;
+ int ret = 0;
+ struct led_init_data init_data = {};
+
+ mcnode = device_get_named_child_node(&pdev->dev, "multi-led");
+ if (!mcnode) {
+ dev_err(&pdev->dev, "expected multi-led node\n");
+ ret = -ENODEV;
+ goto out;
+ }
+
+ /* count the nodes inside the multi-led node */
+ fwnode_for_each_child_node(mcnode, fwnode)
+ ++count;
+
+ priv = devm_kzalloc(&pdev->dev, struct_size(priv, leds, count),
+ GFP_KERNEL);
+ if (!priv) {
+ ret = -ENOMEM;
+ goto out;
+ }
+ mutex_init(&priv->lock);
+
+ subled = devm_kcalloc(&pdev->dev, count, sizeof(*subled), GFP_KERNEL);
+ if (!subled) {
+ ret = -ENOMEM;
+ goto destroy_mutex;
+ }
+ priv->mc_cdev.subled_info = subled;
+
+ /* init the multicolor's LED class device */
+ cdev = &priv->mc_cdev.led_cdev;
+ fwnode_property_read_string(mcnode, "label", &cdev->name);
+ cdev->brightness = LED_OFF;
+ fwnode_property_read_u32(mcnode, "max-brightness",
+ &cdev->max_brightness);
+ cdev->flags = LED_CORE_SUSPENDRESUME;
+ cdev->brightness_set_blocking = led_pwm_mc_set;
+
+ /* iterate over the nodes inside the multi-led node */
+ fwnode_for_each_child_node(mcnode, fwnode) {
+ pwmled = &priv->leds[priv->mc_cdev.num_colors];
+ pwmled->pwm = devm_fwnode_pwm_get(&pdev->dev, fwnode, NULL);
+ if (IS_ERR(pwmled->pwm)) {
+ ret = PTR_ERR(pwmled->pwm);
+ dev_err(&pdev->dev, "unable to request PWM: %d\n", ret);
+ goto destroy_mutex;
+ }
+ pwm_init_state(pwmled->pwm, &pwmled->pwmstate);
+
+ ret = fwnode_property_read_u32(fwnode, "color", &color);
+ if (ret) {
+ dev_err(&pdev->dev, "cannot read color: %d\n", ret);
+ goto destroy_mutex;
+ }
+
+ subled[priv->mc_cdev.num_colors].color_index = color;
+ subled[priv->mc_cdev.num_colors].channel =
+ priv->mc_cdev.num_colors;
+ ++priv->mc_cdev.num_colors;
+ }
+
+ init_data.fwnode = mcnode;
+ ret = devm_led_classdev_multicolor_register_ext(&pdev->dev,
+ &priv->mc_cdev,
+ &init_data);
+ if (ret) {
+ dev_err(&pdev->dev,
+ "failed to register multicolor PWM led for %s: %d\n",
+ cdev->name, ret);
+ goto destroy_mutex;
+ }
+
+ ret = led_pwm_mc_set(cdev, cdev->brightness);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to set led PWM value for %s: %d",
+ cdev->name, ret);
+ goto destroy_mutex;
+ }
+
+ platform_set_drvdata(pdev, priv);
+ return 0;
+
+destroy_mutex:
+ mutex_destroy(&priv->lock);
+out:
+ return ret;
+}
+
+static int led_pwm_mc_remove(struct platform_device *pdev)
+{
+ struct pwm_mc_led *priv = platform_get_drvdata(pdev);
+
+ mutex_destroy(&priv->lock);
+ return 0;
+}
+
+static const struct of_device_id of_pwm_leds_mc_match[] = {
+ { .compatible = "pwm-leds-multicolor", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, of_pwm_leds_mc_match);
+
+static struct platform_driver led_pwm_mc_driver = {
+ .probe = led_pwm_mc_probe,
+ .remove = led_pwm_mc_remove,
+ .driver = {
+ .name = "leds_pwm_multicolor",
+ .of_match_table = of_pwm_leds_mc_match,
+ },
+};
+
+module_platform_driver(led_pwm_mc_driver);
+
+MODULE_AUTHOR("Sven Schwermer <sven.schwermer@disruptive-technologies.com>");
+MODULE_DESCRIPTION("multi-color PWM LED driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:leds-pwm-multicolor");
--
2.35.0
^ permalink raw reply related
* [RFC PATCH v2 0/2] Multicolor PWM LED support
From: sven @ 2022-01-25 15:12 UTC (permalink / raw)
To: linux-leds, devicetree, linux-pwm
Cc: Sven Schwermer, pavel, dmurphy, robh+dt, thierry.reding,
u.kleine-koenig, lee.jones, post
From: Sven Schwermer <sven.schwermer@disruptive-technologies.com>
Hi,
As previously discussed [1] on the linux-leds list I am missing
multicolor PWM LED support. In the mean time I have put together a
working prototype for such a driver. This is my first Linux driver
so I'm hoping for some feedback. Here are some questions that came up
while putting this thing together:
1. Currently, the max-brightness property is expected as a property to
the multi-led node. That seems consistent with the existing
multicolor class code, but I'm wondering whether it would make
sense to have a max-brigthness for the individual LEDs as well?
2. The current multi-led node definition calls for a node index which
would in turn require the reg property to be set within the node.
In this context, that doesn't seem to make sense. Should this
requirement be lifted from leds-class-multicolor.yaml?
3. I'm not currently reusing any leds-pwm code because there aren't
too many overlaps. Does anyone have suggestions what could be
factored out into a common source file?
I would appreciate if anyone would test this code. It runs on my
i.MX6ULL-based hardware.
Best regards,
Sven
[1]: https://www.spinics.net/lists/linux-leds/msg19988.html
Sven Schwermer (2):
dt-bindings: leds: Add multicolor PWM LED bindings
leds: Add PWM multicolor driver
.../bindings/leds/leds-pwm-multicolor.yaml | 76 ++++++++
drivers/leds/Kconfig | 8 +
drivers/leds/Makefile | 1 +
drivers/leds/leds-pwm-multicolor.c | 184 ++++++++++++++++++
4 files changed, 269 insertions(+)
create mode 100644 Documentation/devicetree/bindings/leds/leds-pwm-multicolor.yaml
create mode 100644 drivers/leds/leds-pwm-multicolor.c
Interdiff against v1:
diff --git a/Documentation/devicetree/bindings/leds/leds-pwm-multicolor.yaml b/Documentation/devicetree/bindings/leds/leds-pwm-multicolor.yaml
index 8552a5498bdd..b82b26f2e140 100644
--- a/Documentation/devicetree/bindings/leds/leds-pwm-multicolor.yaml
+++ b/Documentation/devicetree/bindings/leds/leds-pwm-multicolor.yaml
@@ -14,7 +14,8 @@ description: |
LED using the multicolor LED class.
properties:
- compatible: pwm-leds-multicolor
+ compatible:
+ const: pwm-leds-multicolor
patternProperties:
'^multi-led@[0-9a-f]$':
@@ -34,10 +35,12 @@ patternProperties:
color:
$ref: common.yaml#/properties/color
+ required:
+ - pwms
+ - color
+
required:
- compatible
- - pwms
- - color
additionalProperties: false
--
2.35.0
^ permalink raw reply related
* [RFC PATCH v2 1/2] dt-bindings: leds: Add multicolor PWM LED bindings
From: sven @ 2022-01-25 15:12 UTC (permalink / raw)
To: linux-leds, devicetree, linux-pwm
Cc: Sven Schwermer, pavel, dmurphy, robh+dt, thierry.reding,
u.kleine-koenig, lee.jones, post
In-Reply-To: <20220125151226.31049-1-sven@svenschwermer.de>
From: Sven Schwermer <sven.schwermer@disruptive-technologies.com>
This allows to group multiple PWM-connected monochrome LEDs into
multicolor LEDs, e.g. RGB LEDs.
Signed-off-by: Sven Schwermer <sven.schwermer@disruptive-technologies.com>
---
.../bindings/leds/leds-pwm-multicolor.yaml | 76 +++++++++++++++++++
1 file changed, 76 insertions(+)
create mode 100644 Documentation/devicetree/bindings/leds/leds-pwm-multicolor.yaml
diff --git a/Documentation/devicetree/bindings/leds/leds-pwm-multicolor.yaml b/Documentation/devicetree/bindings/leds/leds-pwm-multicolor.yaml
new file mode 100644
index 000000000000..b82b26f2e140
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/leds-pwm-multicolor.yaml
@@ -0,0 +1,76 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/leds-pwm-multicolor.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Multi-color LEDs connected to PWM
+
+maintainers:
+ - Sven Schwermer <sven.schwermer@disruptive-technologies.com>
+
+description: |
+ This driver combines several monochrome PWM LEDs into one multi-color
+ LED using the multicolor LED class.
+
+properties:
+ compatible:
+ const: pwm-leds-multicolor
+
+patternProperties:
+ '^multi-led@[0-9a-f]$':
+ type: object
+ allOf:
+ - $ref: leds-class-multicolor.yaml#
+
+ patternProperties:
+ "^led-[0-9a-z]+$":
+ type: object
+ properties:
+ pwms:
+ maxItems: 1
+
+ pwm-names: true
+
+ color:
+ $ref: common.yaml#/properties/color
+
+ required:
+ - pwms
+ - color
+
+required:
+ - compatible
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/leds/common.h>
+
+ rgb-led {
+ compatible = "pwm-leds-multicolor";
+
+ multi-led@0 {
+ color = <LED_COLOR_ID_RGB>;
+ function = LED_FUNCTION_INDICATOR;
+ max-brightness = <65535>;
+
+ led-red {
+ pwms = <&pwm1 0 1000000>;
+ color = <LED_COLOR_ID_RED>;
+ };
+
+ led-green {
+ pwms = <&pwm2 0 1000000>;
+ color = <LED_COLOR_ID_GREEN>;
+ };
+
+ led-blue {
+ pwms = <&pwm3 0 1000000>;
+ color = <LED_COLOR_ID_BLUE>;
+ };
+ };
+ };
+
+...
--
2.35.0
^ permalink raw reply related
* Re: [PATCH 0/3] arm64: dts: meson: add BL32 reserved region to Beelink g12b devices
From: Neil Armstrong @ 2022-01-25 15:01 UTC (permalink / raw)
To: Christian Hewitt, Kevin Hilman
Cc: Rob Herring, Mark Rutland, devicetree, linux-arm-kernel,
linux-amlogic, linux-kernel, Furkan Kardame
In-Reply-To: <C8B4EA0E-6593-4C42-B116-F9C043D29452@gmail.com>
Hi,
On 25/01/2022 05:02, Christian Hewitt wrote:
>
>> On 25 Jan 2022, at 12:02 am, Kevin Hilman <khilman@baylibre.com <mailto:khilman@baylibre.com>> wrote:
>>
>> Christian Hewitt <christianshewitt@gmail.com <mailto:christianshewitt@gmail.com>> writes:
>>
>>> This resolves a long-running issue where Beelink GT-King/Pro and
>>> GS-King-X wedge on boot or shortly after when booting from vendor
>>> u-boot. In some distros the issue is often reported as triggered
>>> by large file transfers to/from USB or SD cards. Reserving the
>>> BL32 memory region prevents the issue.
>>
>> The BL32 is typically common for the SoC family, so this change should
>> probably go into the g12b.dtsi. Or probably even
>> meson-g12-common.dtsi, which is where the BL31 reserved-memory is
>> described.
>
> Hi Kevin,
>
> Would you be okay with the same change applied to GX devices too? - I
> normally have these two catch-all patches in my tree to deal with random
> tv box hardware and it would be great to drop them:
>
> https://github.com/chewitt/linux/commit/4315ea4612389fc08d0a008b562cafbda96374fc <https://github.com/chewitt/linux/commit/4315ea4612389fc08d0a008b562cafbda96374fc>
> https://github.com/chewitt/linux/commit/3c0df794baa7ea9d32d8ad54530b5a056c770ea9 <https://github.com/chewitt/linux/commit/3c0df794baa7ea9d32d8ad54530b5a056c770ea9>
Sure, it has been done in a similar way for bl31:
https://github.com/torvalds/linux/commit/48e21ded0432ee1e2359d4143d7a6925cefee1b5
in a perfect work we wouldn't need this since mainline U-Boot does the job by reserving these
memory zones, but vendor u-boot exists and isn't avoidable.
Neil
>
> Christian
^ permalink raw reply
* Re: [PATCH 6/8] nvmem: transformations: ethernet address offset support
From: Michael Walle @ 2022-01-25 14:59 UTC (permalink / raw)
To: Rafał Miłecki
Cc: linux-mtd, devicetree, linux-kernel, linux-arm-kernel, netdev,
Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
Rob Herring, Srinivas Kandagatla, Shawn Guo, Li Yang,
Frank Rowand, David S . Miller, Jakub Kicinski, Ansuel Smith,
Andrew Lunn
In-Reply-To: <455f4360-34fe-7fee-66d5-fd945fe1e086@gmail.com>
Hi,
Am 2022-01-25 13:08, schrieb Rafał Miłecki:
> On 28.12.2021 15:25, Michael Walle wrote:
>> An nvmem cell might just contain a base MAC address. To generate a
>> address of a specific interface, add a transformation to add an offset
>> to this base address.
>>
>> Add a generic implementation and the first user of it, namely the sl28
>> vpd storage.
>>
>> Signed-off-by: Michael Walle <michael@walle.cc>
>> ---
>> drivers/nvmem/transformations.c | 45
>> +++++++++++++++++++++++++++++++++
>> 1 file changed, 45 insertions(+)
>>
>> diff --git a/drivers/nvmem/transformations.c
>> b/drivers/nvmem/transformations.c
>> index 61642a9feefb..15cd26da1f83 100644
>> --- a/drivers/nvmem/transformations.c
>> +++ b/drivers/nvmem/transformations.c
>> @@ -12,7 +12,52 @@ struct nvmem_transformations {
>> nvmem_cell_post_process_t pp;
>> };
>> +/**
>> + * nvmem_transform_mac_address_offset() - Add an offset to a mac
>> address cell
>> + *
>> + * A simple transformation which treats the index argument as an
>> offset and add
>> + * it to a mac address. This is useful, if the nvmem cell stores a
>> base
>> + * ethernet address.
>> + *
>> + * @index: nvmem cell index
>> + * @data: nvmem data
>> + * @bytes: length of the data
>> + *
>> + * Return: 0 or negative error code on failure.
>> + */
>> +static int nvmem_transform_mac_address_offset(int index, unsigned int
>> offset,
>> + void *data, size_t bytes)
>> +{
>> + if (bytes != ETH_ALEN)
>> + return -EINVAL;
>> +
>> + if (index < 0)
>> + return -EINVAL;
>> +
>> + if (!is_valid_ether_addr(data))
>> + return -EINVAL;
>> +
>> + eth_addr_add(data, index);
>> +
>> + return 0;
>> +}
>> +
>> +static int nvmem_kontron_sl28_vpd_pp(void *priv, const char *id, int
>> index,
>> + unsigned int offset, void *data,
>> + size_t bytes)
>> +{
>> + if (!id)
>> + return 0;
>> +
>> + if (!strcmp(id, "mac-address"))
>> + return nvmem_transform_mac_address_offset(index, offset, data,
>> + bytes);
>> +
>> + return 0;
>> +}
>> +
>> static const struct nvmem_transformations nvmem_transformations[] =
>> {
>> + { .compatible = "kontron,sl28-vpd", .pp = nvmem_kontron_sl28_vpd_pp
>> },
>> {}
>> };
>
> I think it's a rather bad solution that won't scale well at all.
>
> You'll end up with a lot of NVMEM device specific strings and code in a
> NVMEM core.
They must not be in the core, but they have to be somewhere. That is
because Rob isn't fond of describing the actual transformation in
the device tree but to have it a specific compatible [1]. Thus you have
to have these strings somewhere in the driver code.
> You'll have a lot of duplicated code (many device specific functions
> calling e.g. nvmem_transform_mac_address_offset()).
If there will be multiple formats using the same transformation for
different compatible strings, you could just use the same function
for all compatibles.
But we have to first agree on the device tree representation because
that is then fixed. The driver code can change over time.
> I think it also ignores fact that one NVMEM device can be reused in
> multiple platforms / device models using different (e.g. vendor /
> device
> specific) cells.
>
>
> What if we have:
> 1. Foo company using "kontron,sl28-vpd" with NVMEM cells:
> a. "mac-address"
> b. "mac-address-2"
> c. "mac-address-3"
> 2. Bar company using "kontron,sl28-vpd" with NVMEM cell:
> a. "mac-address"
>
> In the first case you don't want any transformation.
I can't follow you here. The "kontron,sl28-vpd" specifies one
particular format, namely, that there is a base address
rather than individual ones.
> If you consider using transformations for ASCII formats too then it
> causes another conflict issue. Consider two devices:
>
> 1. Foo company device with BIN format of MAC
> 2. Bar company device with ASCII format of MAC
>
> Both may use exactly the same binding:
>
> partition@0 {
> compatible = "nvmem-cells";
> reg = <0x0 0x100000>;
> label = "bootloader";
>
> #address-cells = <1>;
> #size-cells = <1>;
>
> mac-address@100 {
> reg = <0x100 0x6>;
> };
> };
>
> how are you going to handle them with proposed implementation? You
> can't
> support both if you share "nvmem-cells" compatible string.
No, you'd need two different compatible strings. Again, that all boils
down to what the device tree should describe and what not.
But if you have the u-boot environment as an nvmem provider, you already
know that the mac address is in ascii representation and you'd need
to transform it to the kernel representation.
> I think that what can solve those problems is assing "compatible" to
> NVMEM cells.
>
> Let me think about details of that possible solution.
See [2].
-michael
[1]
https://lore.kernel.org/linux-devicetree/YaZ5JNCFeKcdIfu8@robh.at.kernel.org/
[2]
https://lore.kernel.org/linux-devicetree/CAL_JsqL55mZJ6jUyQACer2pKMNDV08-FgwBREsJVgitnuF18Cg@mail.gmail.com/
^ permalink raw reply
* Re: [PATCH v10 0/4] clk: meson: add a sub EMMC clock controller support
From: Neil Armstrong @ 2022-01-25 14:54 UTC (permalink / raw)
To: Liang Yang, Jerome Brunet, Kevin Hilman, Michael Turquette,
Stephen Boyd, Rob Herring, linux-clk
Cc: Martin Blumenstingl, Jianxin Pan, Victor Wan, XianWei Zhao,
Kelvin Zhang, BiChao Zheng, YongHui Yu, linux-arm-kernel,
linux-amlogic, linux-kernel, devicetree
In-Reply-To: <20220121074508.42168-1-liang.yang@amlogic.com>
Hi Liang,
On 21/01/2022 08:45, Liang Yang wrote:
> This driver will add a MMC clock controller driver support.
> The original idea about adding a clock controller is during the
> discussion in the NAND driver mainline effort[1].
>
> This driver is tested in the S400 board (AXG platform) with NAND driver.
Thanks a lot for providing a fixed and updated version of this serie.
After some chat with Jerome and Kevin, it seems the way the eMMC clock reuse
for NAND was designed nearly 4 years doesn't look accurate anymore.
Having a separate clk driver designed to replace the eMMC node when NAND is
used on the board seems over engineered.
Actually having the clock code you add in this serie _but_ directly in
the NAND looks far better, and more coherent since having Linux runtime
detection of eMMC vs NAND will never happen and even this serie required
some DT modification from the bootloader.
I'll let Jerome or Kevin add more details if they want, but I think you should resurrect
the work you pushed in [1] & [2] but:
- passing the eMMC clk registers as a third "reg" cell
- passing the same "clocks" phandle as the eMMC node
- adding the eMMC clock code in the NAND driver directly
I'm open to discussions if you consider the current approach is still superior.
Thanks,
Neil
[1] https://lore.kernel.org/r/20220106033130.37623-1-liang.yang@amlogic.com
[2] https://lore.kernel.org/r/20220106032504.23310-1-liang.yang@amlogic.com
>
> Changes since v9 [10]
> - use clk_parent_data instead of parent_names
>
> Changes since v8 [9]
> - use MESON_SCLK_ONE_BASED instead of CLK_DIVIDER_ONE_BASED
> - use struct_size to caculate onecell_data
> - add clk-phase-delay.h
> - define CLK_DELAY_STEP_PS_GX and CLK_DELAY_STEP_PS_AXG
>
> Changes since v7 [8]
> - move meson_clk_get_phase_delay_data() from header to driver
> - CONFIG sclk-div with COMMON_CLK_AMLOGIC instead of COMMON_CLK_AMLOGIC_AUDIO
> - remove onecell date and ID for internal MUX clk
> - use helper for functions for ONE_BASED in sclk-div
> - add ONE_BASED support for duty cycle
>
> Changes since v6 [7]:
> - add one based support for sclk divier
> - alloc sclk in probe for multiple instance
> - fix coding styles
>
> Changes since v5 [6]:
> - remove divider ops with .init and use sclk_div instead
> - drop CLK_DIVIDER_ROUND_CLOSEST in mux and div
> - drop the useless type cast
>
> Changes since v4 [5]:
> - use struct parm in phase delay driver
> - remove 0 delay releted part in phase delay driver
> - don't rebuild the parent name once again
> - add divider ops with .init
>
> Changes since v3 [4]:
> - separate clk-phase-delay driver
> - replace clk_get_rate() with clk_hw_get_rate()
> - collect Rob's R-Y
> - drop 'meson-' prefix from compatible string
>
> Changes since v2 [3]:
> - squash dt-binding clock-id patch
> - update license
> - fix alignment
> - construct a clk register helper() function
>
> Changes since v1 [2]:
> - implement phase clock
> - update compatible name
> - adjust file name
> - divider probe() into small functions, and re-use them
>
> [1] https://lkml.kernel.org/r/20180628090034.0637a062@xps13
> [2] https://lkml.kernel.org/r/20180703145716.31860-1-yixun.lan@amlogic.com
> [3] https://lkml.kernel.org/r/20180710163658.6175-1-yixun.lan@amlogic.com
> [4] https://lkml.kernel.org/r/20180712211244.11428-1-yixun.lan@amlogic.com
> [5] https://lkml.kernel.org/r/20180809070724.11935-4-yixun.lan@amlogic.com
> [6] https://lkml.kernel.org/r/1539839245-13793-1-git-send-email-jianxin.pan@amlogic.com
> [7] https://lkml.kernel.org/r/1541089855-19356-1-git-send-email-jianxin.pan@amlogic.com
> [8] https://lkml.kernel.org/r/1544457877-51301-1-git-send-email-jianxin.pan@amlogic.com
> [9] https://lkml.kernel.org/r/1545063850-21504-1-git-send-email-jianxin.pan@amlogic.com
> [10] https://lore.kernel.org/all/20220113115745.45826-1-liang.yang@amlogic.com/
> Liang Yang (4):
> clk: meson: add one based divider support for sclk
> clk: meson: add emmc sub clock phase delay driver
> clk: meson: add DT documentation for emmc clock controller
> clk: meson: add sub MMC clock controller driver
>
> .../bindings/clock/amlogic,mmc-clkc.yaml | 64 ++++
> drivers/clk/meson/Kconfig | 18 ++
> drivers/clk/meson/Makefile | 2 +
> drivers/clk/meson/clk-phase-delay.c | 69 ++++
> drivers/clk/meson/clk-phase-delay.h | 20 ++
> drivers/clk/meson/mmc-clkc.c | 302 ++++++++++++++++++
> drivers/clk/meson/sclk-div.c | 59 ++--
> drivers/clk/meson/sclk-div.h | 3 +
> include/dt-bindings/clock/amlogic,mmc-clkc.h | 14 +
> 9 files changed, 529 insertions(+), 22 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/clock/amlogic,mmc-clkc.yaml
> create mode 100644 drivers/clk/meson/clk-phase-delay.c
> create mode 100644 drivers/clk/meson/clk-phase-delay.h
> create mode 100644 drivers/clk/meson/mmc-clkc.c
> create mode 100644 include/dt-bindings/clock/amlogic,mmc-clkc.h
>
^ permalink raw reply
* Re: [PATCH v13, 2/2] net: Add dm9051 driver
From: Andy Shevchenko @ 2022-01-25 14:50 UTC (permalink / raw)
To: Joseph CHAMG
Cc: David S . Miller, Jakub Kicinski, Rob Herring, joseph_chang,
netdev, devicetree, Linux Kernel Mailing List, Andrew Lunn,
Leon Romanovsky
In-Reply-To: <20220125085837.10357-3-josright123@gmail.com>
On Tue, Jan 25, 2022 at 10:59 AM Joseph CHAMG <josright123@gmail.com> wrote:
>
> Add davicom dm9051 spi ethernet driver. The driver work for the
> device platform with spi master
This is better, but please use a grammar period as well.
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Cc: Leon Romanovsky <leon@kernel.org>
> Cc: andy Shevchenko <andy.shevchenko@gmail.com>
Andy
And you may utilize --cc parameter to git send-email or move this Cc
block behind the cutter '--- ' line.
...
> +#include <linux/etherdevice.h>
> +#include <linux/ethtool.h>
> +#include <linux/interrupt.h>
> +#include <linux/iopoll.h>
> +#include <linux/mii.h>
> +#include <linux/module.h>
> +#include <linux/netdevice.h>
> +#include <linux/phy.h>
> +#include <linux/regmap.h>
> +#include <linux/skbuff.h>
> +#include <linux/spinlock.h>
> +#include <linux/spi/spi.h>
types.h is missing
...
> +/**
> + * struct rx_ctl_mach - rx activities record
> + *
In all similar cases remove this blank line.
> + * @status_err_counter: rx status error counter
> + * @large_err_counter: rx get large packet length error counter
> + * @fifo_rst_counter: reset operation counter
> + *
> + * To keep track for the driver operation statistics
> + */
...
> +/**
> + * struct dm9051_rxhdr - rx packet data header
> + *
> + * @rxpktready: lead byte is 0x01 tell a valid packet
> + * @rxstatus: status bits for the received packet
> + * @rxlen: packet length
> + *
> + * The rx packet enter into the fifo memory is start with these four
The Rx packed, entered into the FIFO memory, starts with
> + * bytes which is the rx header, follow this header is the ethernet
Rx header, followed by the ethernet
> + * packet data and end with a appended 4-byte CRC data.
ends
an appended
> + * Both rx packet and CRC data are for check purpose and finally are
> + * dropped by this driver
> + */
...
> + * @kw_rxctrl: kernel thread worke structure for rx control
worker?
...
> + int ret = regmap_read_poll_timeout(db->regmap_dm, DM9051_NSR, mval,
> + mval & (NSR_TX2END | NSR_TX1END), 1, 20);
> +
> + if (ret)
Please, split the assignment and get it closer to its user, so
int ret;
ret = ...
if (ret)
This applies to all similar cases.
Actually all comments are against the entire code even if it's given
only for one occurrence of the similar code block.
> + netdev_err(db->ndev, "timeout in checking for tx ends\n");
> + return ret;
> +}
...
> + ret = regmap_bulk_read(db->regmap_dmbulk, DM9051_EPDRL, to, 2);
> + if (ret < 0)
> + return ret;
> + return ret;
return regmap_...(...);
Same for other similar places.
...
> + /* this is a 2 bytes data written via regmap_bulk_write
> + */
Useless comments.
...
> +static int dm9051_mdiobus_read(struct mii_bus *mdiobus, int phy_id, int reg)
> +{
> + struct board_info *db = mdiobus->priv;
> + unsigned int val = 0;
You can do
val = 0xffff;
here...
> + int ret;
> +
> + if (phy_id == DM9051_PHY_ID) {
> + ret = ctrl_dm9051_phyread(db, reg, &val);
> + if (ret)
> + return ret;
> + return val;
> + }
> + return 0xffff;
...and
}
return val;
here.
> +}
> + while (count--) {
If the count is guaranteed to be greater than 0, it would be better to use
do {
...
} while (--count);
> + ret = regmap_read(db->regmap_dm, reg, &rb);
> + if (ret) {
> + netif_err(db, drv, ndev, "%s: error %d dumping read reg %02x\n",
> + __func__, ret, reg);
> + break;
> + }
> + }
> + return ret;
> +}
...
> +#ifndef _DM9051_H_
> +#define _DM9051_H_
> +
> +#include <linux/bitfield.h>
There is no user of this header, but missing bits.h and one that
provides netdev_priv().
...
> +#define DRVNAME_9051 "dm9051"
Why is this in the header?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v6 0/2] Add spi control driver for Sunplus SP7021 SoC
From: Mark Brown @ 2022-01-25 14:35 UTC (permalink / raw)
To: linux-kernel, devicetree, linux-spi, Li-hao Kuo, p.zabel, robh+dt,
andyshevchenko
Cc: lh.kuo, wells.lu
In-Reply-To: <cover.1642494310.git.lhjeff911@gmail.com>
On Tue, 18 Jan 2022 16:42:37 +0800, Li-hao Kuo wrote:
> This is a patch series for SPI driver for Sunplus SP7021 SoC.
>
> Sunplus SP7021 is an ARM Cortex A7 (4 cores) based SoC. It integrates
> many peripherals (ex: UART, I2C, SPI, SDIO, eMMC, USB, SD card and
> etc.) into a single chip. It is designed for industrial control.
>
> Refer to:
> https://sunplus-tibbo.atlassian.net/wiki/spaces/doc/overview
> https://tibbo.com/store/plus1.html
>
> [...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
Thanks!
[1/2] spi: Add spi driver for Sunplus SP7021
commit: f62ca4e2a863033d9b3b5a00a0d897557c9da6c5
[2/2] dt-bindings:spi: Add Sunplus SP7021 schema
(no commit info)
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
^ permalink raw reply
* Re: [PATCH v2 10/12] arm64: dts: renesas: Add initial DTSI for RZ/V2L SoC
From: Geert Uytterhoeven @ 2022-01-25 14:23 UTC (permalink / raw)
To: Rob Herring
Cc: Lad Prabhakar, Linux-Renesas, Magnus Damm, Biju Das, Prabhakar,
Linux Kernel Mailing List,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
In-Reply-To: <CAL_JsqKnWwSEneKTvQWWmGk1gJG2dx37vAJAFWOLVm5wL5t31g@mail.gmail.com>
Hi Rob,
On Tue, Jan 25, 2022 at 3:00 PM Rob Herring <robh+dt@kernel.org> wrote:
> On Mon, Jan 10, 2022 at 7:47 AM Lad Prabhakar
> <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> > From: Biju Das <biju.das.jz@bp.renesas.com>
> >
> > The RZ/V2L is package- and pin-compatible with the RZ/G2L. The only
> > difference being the RZ/V2L SoC has additional DRP-AI IP (AI
> > accelerator).
> >
> > Add initial DTSI for RZ/V2L SoC with below SoC specific dtsi files for
> > supporting single core and dual core devices.
> >
> > r9a07g054l1.dtsi => RZ/V2L R9A07G054L1 SoC specific parts
> > r9a07g054l2.dtsi => RZ/V2L R9A07G054L2 SoC specific parts
> >
> > Both RZ/G2L and RZ/V2L SMARC EVK SoM are identical apart from SoC's
> > used hence the common dtsi files (rzg2l-smarc*.dtsi) are share between
> > RZ/G2L and RZ/V2L SMARC EVK. Place holders are added in device nodes to
> > avoid compilation errors for the devices which have not been enabled yet
> > on RZ/V2L SoC.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/renesas/r9a07g054.dtsi
> > @@ -0,0 +1,491 @@
> > +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +/*
> > + * Device Tree Source for the RZ/V2L SoC
> > + *
> > + * Copyright (C) 2021 Renesas Electronics Corp.
> > + */
> > +
> > +#include <dt-bindings/interrupt-controller/arm-gic.h>
> > +#include <dt-bindings/clock/r9a07g054-cpg.h>
>
> linux-next is failing because this header is missing:
>
> In file included from arch/arm64/boot/dts/renesas/r9a07g054l2.dtsi:9,
> from arch/arm64/boot/dts/renesas/r9a07g054l2-smarc.dts:9:
> arch/arm64/boot/dts/renesas/r9a07g054.dtsi:9:10: fatal error:
> dt-bindings/clock/r9a07g054-cpg.h: No such file or directory
> 9 | #include <dt-bindings/clock/r9a07g054-cpg.h>
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Thanks, I have already removed the offending commits from renesas-next.
as the header is not ready yet.
Interestingly, kernel test robot reported a success for that branch...
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [RFC PATCH 1/2] dt-bindings: leds: Add multicolor PWM LED bindings
From: Rob Herring @ 2022-01-25 14:25 UTC (permalink / raw)
To: sven
Cc: devicetree, thierry.reding, dmurphy, Sven Schwermer, robh+dt,
u.kleine-koenig, linux-leds, linux-pwm, lee.jones, pavel
In-Reply-To: <20220125092239.2006333-2-sven@svenschwermer.de>
On Tue, 25 Jan 2022 10:22:38 +0100, sven@svenschwermer.de wrote:
> From: Sven Schwermer <sven.schwermer@disruptive-technologies.com>
>
> This allows to group multiple PWM-connected monochrome LEDs into
> multicolor LEDs, e.g. RGB LEDs.
>
> Signed-off-by: Sven Schwermer <sven.schwermer@disruptive-technologies.com>
> ---
> .../bindings/leds/leds-pwm-multicolor.yaml | 73 +++++++++++++++++++
> 1 file changed, 73 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/leds/leds-pwm-multicolor.yaml
>
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/leds/leds-pwm-multicolor.yaml: properties:compatible: 'pwm-leds-multicolor' is not of type 'object', 'boolean'
from schema $id: http://json-schema.org/draft-07/schema#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/leds/leds-pwm-multicolor.yaml: ignoring, error in schema: properties: compatible
Documentation/devicetree/bindings/leds/leds-pwm-multicolor.example.dts:24.25-43.15: Warning (unit_address_vs_reg): /example-0/rgb-led/multi-led@0: node has a unit name, but no reg or ranges property
Documentation/devicetree/bindings/leds/leds-pwm-multicolor.example.dt.yaml:0:0: /example-0/rgb-led: failed to match any schema with compatible: ['pwm-leds-multicolor']
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/patch/1583948
This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit.
^ permalink raw reply
* Re: [PATCH 2/2] arm64: dts: qcom: sm8450: fix apps_smmu interrupts
From: Vinod Koul @ 2022-01-25 14:20 UTC (permalink / raw)
To: Jonathan Marek
Cc: linux-arm-msm, Andy Gross, Bjorn Andersson, Rob Herring,
Konrad Dybcio,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
In-Reply-To: <20220122162932.7686-2-jonathan@marek.ca>
On 22-01-22, 11:29, Jonathan Marek wrote:
> Update interrupts in apps_smmu to match downstream. This is fixes apps_smmu
> failing to probe when running at EL2 (expects 96 context interrupts)
Reviewed-by: Vinod Koul <vkoul@kernel.org>
--
~Vinod
^ permalink raw reply
* Re: [PATCH 1/2] arm64: dts: qcom: sm8450: enable GCC_USB3_0_CLKREF_EN for usb
From: Vinod Koul @ 2022-01-25 14:20 UTC (permalink / raw)
To: Jonathan Marek
Cc: linux-arm-msm, Andy Gross, Bjorn Andersson, Rob Herring,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
In-Reply-To: <20220122162932.7686-1-jonathan@marek.ca>
On 22-01-22, 11:29, Jonathan Marek wrote:
> USB doesn't work at all without this clock enabled. This fixes USB when not
> using clk_ignore_unused.
Reviewed-by: Vinod Koul <vkoul@kernel.org>
--
~Vinod
^ permalink raw reply
* Re: [PATCH v2 10/12] arm64: dts: renesas: Add initial DTSI for RZ/V2L SoC
From: Rob Herring @ 2022-01-25 14:00 UTC (permalink / raw)
To: Lad Prabhakar
Cc: Geert Uytterhoeven, linux-renesas-soc, Magnus Damm, Biju Das,
Prabhakar, linux-kernel, devicetree
In-Reply-To: <20220110134659.30424-11-prabhakar.mahadev-lad.rj@bp.renesas.com>
On Mon, Jan 10, 2022 at 7:47 AM Lad Prabhakar
<prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
>
> From: Biju Das <biju.das.jz@bp.renesas.com>
>
> The RZ/V2L is package- and pin-compatible with the RZ/G2L. The only
> difference being the RZ/V2L SoC has additional DRP-AI IP (AI
> accelerator).
>
> Add initial DTSI for RZ/V2L SoC with below SoC specific dtsi files for
> supporting single core and dual core devices.
>
> r9a07g054l1.dtsi => RZ/V2L R9A07G054L1 SoC specific parts
> r9a07g054l2.dtsi => RZ/V2L R9A07G054L2 SoC specific parts
>
> Both RZ/G2L and RZ/V2L SMARC EVK SoM are identical apart from SoC's
> used hence the common dtsi files (rzg2l-smarc*.dtsi) are share between
> RZ/G2L and RZ/V2L SMARC EVK. Place holders are added in device nodes to
> avoid compilation errors for the devices which have not been enabled yet
> on RZ/V2L SoC.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> v1->v2
> * None
> ---
> arch/arm64/boot/dts/renesas/r9a07g054.dtsi | 491 +++++++++++++++++++
> arch/arm64/boot/dts/renesas/r9a07g054l1.dtsi | 25 +
> arch/arm64/boot/dts/renesas/r9a07g054l2.dtsi | 13 +
> 3 files changed, 529 insertions(+)
> create mode 100644 arch/arm64/boot/dts/renesas/r9a07g054.dtsi
> create mode 100644 arch/arm64/boot/dts/renesas/r9a07g054l1.dtsi
> create mode 100644 arch/arm64/boot/dts/renesas/r9a07g054l2.dtsi
>
> diff --git a/arch/arm64/boot/dts/renesas/r9a07g054.dtsi b/arch/arm64/boot/dts/renesas/r9a07g054.dtsi
> new file mode 100644
> index 000000000000..5de8f343f12a
> --- /dev/null
> +++ b/arch/arm64/boot/dts/renesas/r9a07g054.dtsi
> @@ -0,0 +1,491 @@
> +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +/*
> + * Device Tree Source for the RZ/V2L SoC
> + *
> + * Copyright (C) 2021 Renesas Electronics Corp.
> + */
> +
> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +#include <dt-bindings/clock/r9a07g054-cpg.h>
linux-next is failing because this header is missing:
In file included from arch/arm64/boot/dts/renesas/r9a07g054l2.dtsi:9,
from arch/arm64/boot/dts/renesas/r9a07g054l2-smarc.dts:9:
arch/arm64/boot/dts/renesas/r9a07g054.dtsi:9:10: fatal error:
dt-bindings/clock/r9a07g054-cpg.h: No such file or directory
9 | #include <dt-bindings/clock/r9a07g054-cpg.h>
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
^ permalink raw reply
* Re: [PATCH v13, 2/2] net: Add dm9051 driver
From: Andrew Lunn @ 2022-01-25 13:59 UTC (permalink / raw)
To: Joseph CHAMG
Cc: David S . Miller, Jakub Kicinski, Rob Herring, joseph_chang,
netdev, devicetree, linux-kernel, andy.shevchenko, leon
In-Reply-To: <20220125085837.10357-3-josright123@gmail.com>
> +static int dm9051_mdiobus_read(struct mii_bus *mdiobus, int phy_id, int reg)
> +{
> + struct board_info *db = mdiobus->priv;
> + unsigned int val = 0;
> + int ret;
> +
> + if (phy_id == DM9051_PHY_ID) {
phy_id is a poor choice of name. It normally means the value you find
in register 2 and 3 of the PHY which identifies the manufacture, make
and possibly revision.
If you look at the read function prototype in struct mii_bus:
https://elixir.bootlin.com/linux/v5.17-rc1/source/include/linux/phy.h#L357
the normal name is addr.
Ideally your driver needs to look similar to other drivers. Ideally
you use the same variable names for the same things. That makes it
easier for somebody else to read your driver and debug it. It makes it
easier to review, etc. It is worth spending time reading a few other
drivers and looking for common patterns, and making use of those
patterns in your driver.
> +static int dm9051_map_phyup(struct board_info *db)
> +{
> + int ret;
> +
> + /* ~BMCR_PDOWN to power-up the internal phy
> + */
> + ret = mdiobus_modify(db->mdiobus, DM9051_PHY_ID, MII_BMCR, BMCR_PDOWN, 0);
> + if (ret < 0)
> + return ret;
You are still touching PHY registers from the MAC driver. Why is your
PHY driver not going this as part of the _config() function?
Andrew
^ 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