* Re: [PATCH] dt-bindings: timer: renesas,tmu: Add R-Car V4M support
From: Conor Dooley @ 2024-04-02 17:49 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Daniel Lezcano, Thomas Gleixner, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Laurent Pinchart, devicetree, linux-renesas-soc,
linux-kernel
In-Reply-To: <8a39386b1a33db6e83e852b3b365bc1adeb25242.1712068574.git.geert+renesas@glider.be>
[-- Attachment #1: Type: text/plain, Size: 280 bytes --]
On Tue, Apr 02, 2024 at 04:37:02PM +0200, Geert Uytterhoeven wrote:
> Document support for the Timer Unit (TMU) in the Renesas R-Car V4M
> (R8A779H0) SoC.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH RFT 01/10] arm64: dts: microchip: sparx5: fix mdio reg
From: Conor Dooley @ 2024-04-02 17:46 UTC (permalink / raw)
To: Steen Hegelund
Cc: Krzysztof Kozlowski, Nicolas Ferre, Claudiu Beznea, Rob Herring,
Krzysztof Kozlowski, Lars Povlsen, Daniel Machon, UNGLinuxDriver,
David S. Miller, Bjarni Jonasson, linux-arm-kernel, devicetree,
linux-kernel
In-Reply-To: <b3d818df8819d2fb3e96fa61b277d49941d9b01b.camel@microchip.com>
[-- Attachment #1: Type: text/plain, Size: 2259 bytes --]
Hey,
On Tue, Apr 02, 2024 at 04:00:32PM +0200, Steen Hegelund wrote:
> On Mon, 2024-04-01 at 17:37 +0200, Krzysztof Kozlowski wrote:
> > [Some people who received this message don't often get email from
> > krzk@kernel.org. Learn why this is important at
> > https://aka.ms/LearnAboutSenderIdentification ]
> >
> > EXTERNAL EMAIL: Do not click links or open attachments unless you
> > know the content is safe
> >
> > Correct the reg address of mdio node to match unit address. Assume
> > the
> > reg is not correct and unit address was correct, because there is
> > alerady node using the existing reg 0x110102d4.
> >
> > sparx5.dtsi:443.25-451.5: Warning (simple_bus_reg):
> > /axi@600000000/mdio@6110102f8: simple-bus unit address format error,
> > expected "6110102d4"
> >
> > Fixes: d0f482bb06f9 ("arm64: dts: sparx5: Add the Sparx5 switch
> > node")
> > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> >
> > ---
> >
> > Not tested on hardware
> > ---
> > arch/arm64/boot/dts/microchip/sparx5.dtsi | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/arm64/boot/dts/microchip/sparx5.dtsi
> > b/arch/arm64/boot/dts/microchip/sparx5.dtsi
> > index 24075cd91420..5d820da8c69d 100644
> > --- a/arch/arm64/boot/dts/microchip/sparx5.dtsi
> > +++ b/arch/arm64/boot/dts/microchip/sparx5.dtsi
> > @@ -447,7 +447,7 @@ mdio2: mdio@6110102f8 {
> > pinctrl-names = "default";
> > #address-cells = <1>;
> > #size-cells = <0>;
> > - reg = <0x6 0x110102d4 0x24>;
> > + reg = <0x6 0x110102f8 0x24>;
> > };
> >
> > mdio3: mdio@61101031c {
> > --
> > 2.34.1
> >
>
> I did a check of our current Sparx5 EVBs and none of them uses
> controller 2 in any revision, so this is probably why it has not come
> up before, so as it stands we have no platform to test this change on
> currently.
>
> Besides that the change looks good to me.
>
> Best Regards
> Steen
>
> Reviewed-by: Steen Hegelund <Steen.Hegelund@microchip.com>
Are you okay with the rest of the series, or have you only looked at
this one patch?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* [PATCH v4 2/2] media: dt-bindings: ovti,ov2680: Document link-frequencies
From: Fabio Estevam @ 2024-04-02 17:40 UTC (permalink / raw)
To: sakari.ailus
Cc: rmfrfs, laurent.pinchart, hansg, robh, krzysztof.kozlowski+dt,
conor+dt, linux-media, devicetree, Fabio Estevam
In-Reply-To: <20240402174028.205434-1-festevam@gmail.com>
From: Fabio Estevam <festevam@denx.de>
Document the link-frequencies property as recommended by the following
document:
https://www.kernel.org/doc/html/v6.9-rc1/driver-api/media/camera-sensor.html#handling-clocks
Signed-off-by: Fabio Estevam <festevam@denx.de>
---
Changes since v3:
- Only document link-frequencies.
.../bindings/media/i2c/ovti,ov2680.yaml | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov2680.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov2680.yaml
index c87677f5e2a2..634d3b821b8c 100644
--- a/Documentation/devicetree/bindings/media/i2c/ovti,ov2680.yaml
+++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov2680.yaml
@@ -50,9 +50,23 @@ properties:
Definition of the regulator used as digital power supply.
port:
- $ref: /schemas/graph.yaml#/properties/port
description:
A node containing an output port node.
+ $ref: /schemas/graph.yaml#/$defs/port-base
+ additionalProperties: false
+
+ properties:
+ endpoint:
+ $ref: /schemas/media/video-interfaces.yaml#
+ additionalProperties: false
+
+ properties:
+ link-frequencies: true
+
+ remote-endpoint: true
+
+ required:
+ - link-frequencies
required:
- compatible
@@ -89,6 +103,7 @@ examples:
port {
ov2680_to_mipi: endpoint {
remote-endpoint = <&mipi_from_sensor>;
+ link-frequencies = /bits/ 64 <330000000>;
};
};
};
--
2.34.1
^ permalink raw reply related
* [PATCH v4 1/2] media: dt-bindings: ovti,ov2680: Fix the power supply names
From: Fabio Estevam @ 2024-04-02 17:40 UTC (permalink / raw)
To: sakari.ailus
Cc: rmfrfs, laurent.pinchart, hansg, robh, krzysztof.kozlowski+dt,
conor+dt, linux-media, devicetree, Fabio Estevam
From: Fabio Estevam <festevam@denx.de>
The original .txt bindings had the OV2680 power supply names correct,
but the transition from .txt to yaml spelled them incorrectly.
Fix the OV2680 power supply names as the original .txt bindings
as these are the names used by the OV2680 driver and in devicetree.
Fixes: 57226cd8c8bf ("media: dt-bindings: ov2680: convert bindings to yaml")
Signed-off-by: Fabio Estevam <festevam@denx.de>
---
Changes since v3:
- Newly introduced.
.../bindings/media/i2c/ovti,ov2680.yaml | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov2680.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov2680.yaml
index cf456f8d9ddc..c87677f5e2a2 100644
--- a/Documentation/devicetree/bindings/media/i2c/ovti,ov2680.yaml
+++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov2680.yaml
@@ -37,15 +37,15 @@ properties:
active low.
maxItems: 1
- dovdd-supply:
+ DOVDD-supply:
description:
Definition of the regulator used as interface power supply.
- avdd-supply:
+ AVDD-supply:
description:
Definition of the regulator used as analog power supply.
- dvdd-supply:
+ DVDD-supply:
description:
Definition of the regulator used as digital power supply.
@@ -59,9 +59,9 @@ required:
- reg
- clocks
- clock-names
- - dovdd-supply
- - avdd-supply
- - dvdd-supply
+ - DOVDD-supply
+ - AVDD-supply
+ - DVDD-supply
- reset-gpios
- port
@@ -82,9 +82,9 @@ examples:
clock-names = "xvclk";
reset-gpios = <&gpio1 3 GPIO_ACTIVE_LOW>;
- dovdd-supply = <&sw2_reg>;
- dvdd-supply = <&sw2_reg>;
- avdd-supply = <®_peri_3p15v>;
+ DOVDD-supply = <&sw2_reg>;
+ DVDD-supply = <&sw2_reg>;
+ AVDD-supply = <®_peri_3p15v>;
port {
ov2680_to_mipi: endpoint {
--
2.34.1
^ permalink raw reply related
* Re: [PATCH v1 1/5] dt-bindings: pwm: Add Loongson PWM controller
From: Rob Herring @ 2024-04-02 17:40 UTC (permalink / raw)
To: Binbin Zhou
Cc: Binbin Zhou, Huacai Chen, Uwe Kleine-König,
Krzysztof Kozlowski, Conor Dooley, Huacai Chen, loongson-kernel,
linux-pwm, devicetree, Xuerui Wang, loongarch
In-Reply-To: <edad2bb5b0045c633734c1499fb163c3c6776121.1711953223.git.zhoubinbin@loongson.cn>
On Tue, Apr 02, 2024 at 03:58:38PM +0800, Binbin Zhou wrote:
> Add Loongson PWM controller binding with DT schema format using
> json-schema.
>
> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> ---
> .../devicetree/bindings/pwm/pwm-loongson.yaml | 64 +++++++++++++++++++
Filename should match compatible.
> MAINTAINERS | 6 ++
> 2 files changed, 70 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/pwm/pwm-loongson.yaml
>
> diff --git a/Documentation/devicetree/bindings/pwm/pwm-loongson.yaml b/Documentation/devicetree/bindings/pwm/pwm-loongson.yaml
> new file mode 100644
> index 000000000000..d25904468353
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pwm/pwm-loongson.yaml
> @@ -0,0 +1,64 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pwm/pwm-loongson.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Loongson PWM Controller
> +
> +maintainers:
> + - Binbin Zhou <zhoubinbin@loongson.cn>
> +
> +description:
> + It is the generic PWM framework driver for Loongson family.
That's describing the driver. Not really relevant to the binding.
> + Each PWM has one pulse width output signal and one pulse input
> + signal to be measured.
> + It can be found on Loongson-2K series cpus and Loongson LS7A bridge chips.
> +
> +allOf:
> + - $ref: pwm.yaml#
> +
> +properties:
> + compatible:
> + oneOf:
> + - const: loongson,ls7a-pwm
> + - items:
> + - enum:
> + - loongson,ls2k0500-pwm
> + - loongson,ls2k1000-pwm
> + - loongson,ls2k2000-pwm
> + - const: loongson,ls7a-pwm
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + clocks:
> + maxItems: 1
> +
> + '#pwm-cells':
> + const: 3
Please define what is in each cell. If there's only 2 signals, then the
first cell defines the output or input (what value for which one?).
Really, the PWM binding is only for outputs, so is a cell even needed? I
suppose we could use it for inputs too, but that's really "input
capture" type operation that timers often have. I'll defer to the PWM
maintainers...
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - clocks
> + - '#pwm-cells'
pwm.yaml makes this required already.
Rob
^ permalink raw reply
* Re: [PATCH 3/3] arm64: dts: qcom: msm8996: Add missing UFS host controller reset
From: Bjorn Andersson @ 2024-04-02 17:35 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Manivannan Sadhasivam, Konrad Dybcio, Alim Akhtar, Avri Altman,
Bart Van Assche, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Andy Gross, Andy Gross, James E.J. Bottomley, Martin K. Petersen,
linux-arm-msm, linux-scsi, devicetree, linux-kernel
In-Reply-To: <CAA8EJpqZYp0C8rT8E=LoVo9fispLNhBn8CEgx1-iMqN_2MQXfg@mail.gmail.com>
On Fri, Feb 09, 2024 at 10:16:25PM +0200, Dmitry Baryshkov wrote:
> On Tue, 30 Jan 2024 at 08:55, Manivannan Sadhasivam
> <manivannan.sadhasivam@linaro.org> wrote:
> >
> > On Mon, Jan 29, 2024 at 11:44:15AM +0200, Dmitry Baryshkov wrote:
> > > On Mon, 29 Jan 2024 at 09:55, Manivannan Sadhasivam
> > > <manivannan.sadhasivam@linaro.org> wrote:
> > > >
> > > > UFS host controller reset is required for the drivers to properly reset the
> > > > controller. Hence, add it.
> > > >
> > > > Fixes: 57fc67ef0d35 ("arm64: dts: qcom: msm8996: Add ufs related nodes")
> > > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > >
> > > I think I had issues previously when I attempted to reset the
> > > controller, but it might be because of the incomplete clocks
> > > programming. Let met check whether it works first.
> > >
> >
> > Sure. Please let me know.
>
> With the clocking fixes in place (I'll send them in a few minutes) and
> with this patch the UFS breaks in the following way:
>
Was this further reviewed/investigated? What would you like me to do
with this patch?
Regards,
Bjorn
^ permalink raw reply
* Re: [PATCH v1 5/5] LoongArch: dts: Add PWM support to Loongson-2K2000
From: kernel test robot @ 2024-04-02 17:34 UTC (permalink / raw)
To: Binbin Zhou, Binbin Zhou, Huacai Chen, Uwe Kleine-König,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: oe-kbuild-all, loongson-kernel, linux-pwm, devicetree,
Xuerui Wang, loongarch
In-Reply-To: <7214b933ce85f9d030828e9efab7fbeb57eb712b.1711953223.git.zhoubinbin@loongson.cn>
Hi Binbin,
kernel test robot noticed the following build errors:
[auto build test ERROR on robh/for-next]
[also build test ERROR on linus/master v6.9-rc2 next-20240402]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Binbin-Zhou/dt-bindings-pwm-Add-Loongson-PWM-controller/20240402-160109
base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
patch link: https://lore.kernel.org/r/7214b933ce85f9d030828e9efab7fbeb57eb712b.1711953223.git.zhoubinbin%40loongson.cn
patch subject: [PATCH v1 5/5] LoongArch: dts: Add PWM support to Loongson-2K2000
config: loongarch-allnoconfig (https://download.01.org/0day-ci/archive/20240403/202404030108.rzArK10u-lkp@intel.com/config)
compiler: loongarch64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240403/202404030108.rzArK10u-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202404030108.rzArK10u-lkp@intel.com/
All errors (new ones prefixed by >>):
>> Error: arch/loongarch/boot/dts/loongson-2k2000.dtsi:123.19-20 syntax error
FATAL ERROR: Unable to parse input tree
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH v2 12/13] ASoC: dt-bindings: davinci-mcbsp: Add the 'ti,T1-framing-{rx/tx}' flags
From: Rob Herring @ 2024-04-02 17:32 UTC (permalink / raw)
To: Bastien Curutchet
Cc: Peter Ujfalusi, Thomas Petazzoni, Liam Girdwood, Takashi Iwai,
linux-kernel, christophercordahi, devicetree, alsa-devel,
Krzysztof Kozlowski, herve.codina, Conor Dooley, Jaroslav Kysela,
linux-sound, Mark Brown
In-Reply-To: <20240402071213.11671-13-bastien.curutchet@bootlin.com>
On Tue, 02 Apr 2024 09:12:12 +0200, Bastien Curutchet wrote:
> McBSP's data delay can be configured from 0 to 2 bit clock periods. 0 is
> used for DSP_B format, 1 for DSP_A format. A data delay of 2 bit clock
> periods can be used to interface to 'T1 framing' devices where data
> stream is preceded by a 'framing bit'. This 2 bit clock data delay is
> not described in the bindings.
>
> Add two flags 'ti,T1-framing-[rx/tx]' to enable a data delay of 2
> bit clock periods in reception or transmission.
>
> Signed-off-by: Bastien Curutchet <bastien.curutchet@bootlin.com>
> ---
> .../devicetree/bindings/sound/davinci-mcbsp.yaml | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v2 1/3] dt-bindings: phy: phy-imx8-pcie: Add binding for i.MX8Q HSIO SerDes PHY
From: Rob Herring @ 2024-04-02 17:21 UTC (permalink / raw)
To: Richard Zhu
Cc: vkoul, kishon, krzysztof.kozlowski+dt, frank.li, conor+dt,
linux-phy, devicetree, linux-arm-kernel, linux-kernel, kernel,
imx
In-Reply-To: <1712036704-21064-2-git-send-email-hongxing.zhu@nxp.com>
On Tue, Apr 02, 2024 at 01:45:02PM +0800, Richard Zhu wrote:
> Add binding for controller ID and HSIO configuration setting of the
> i.MX8Q HSIO SerDes PHY.
>
> Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> ---
> include/dt-bindings/phy/phy-imx8-pcie.h | 29 +++++++++++++++++++++++++
> 1 file changed, 29 insertions(+)
>
> diff --git a/include/dt-bindings/phy/phy-imx8-pcie.h b/include/dt-bindings/phy/phy-imx8-pcie.h
> index 8bbe2d6538d8..3292c8be3354 100644
> --- a/include/dt-bindings/phy/phy-imx8-pcie.h
> +++ b/include/dt-bindings/phy/phy-imx8-pcie.h
> @@ -11,4 +11,33 @@
> #define IMX8_PCIE_REFCLK_PAD_INPUT 1
> #define IMX8_PCIE_REFCLK_PAD_OUTPUT 2
>
> +/*
> + * i.MX8QM HSIO subsystem has three lane PHYs and three controllers:
> + * PCIEA(2 lanes capapble PCIe controller), PCIEB (only support one
capable
> + * lane) and SATA.
> + *
> + * In the different use cases. PCIEA can be binded to PHY lane0, lane1
s/binded/bound/
And throughout your patches.
> + * or Lane0 and lane1. PCIEB can be binded to lane1 or lane2 PHY. SATA
> + * can only be binded to last lane2 PHY.
> + *
> + * Define i.MX8Q HSIO controller ID here to specify the controller
> + * binded to the PHY.
> + * Meanwhile, i.MX8QXP HSIO subsystem has one lane PHY and PCIEB(only
> + * support one lane) controller.
> + */
> +#define IMX8Q_HSIO_PCIEA_ID 0
> +#define IMX8Q_HSIO_PCIEB_ID 1
> +#define IMX8Q_HSIO_SATA_ID 2
Please use the standard phy mode defines.
> +
> +/*
> + * On i.MX8QM, PCIEA is mandatory required if the HSIO is enabled.
> + * Define configurations beside PCIEA is enabled.
> + *
> + * On i.MX8QXP, HSIO module only has PCIEB and one lane PHY.
> + * The "IMX8Q_HSIO_CFG_PCIEB" can be used on i.MX8QXP platforms.
> + */
> +#define IMX8Q_HSIO_CFG_SATA 1
> +#define IMX8Q_HSIO_CFG_PCIEB 2
> +#define IMX8Q_HSIO_CFG_PCIEBSATA 3
This seems somewhat redundant both as the 3rd define is just an OR of
the first 2 and all 3 overlap with the prior defines.
Seems like with standard PHY modes, the only additional information you
might need is PCIEB vs. PCIEA.
Rob
^ permalink raw reply
* Re: [PATCH] dt-bindings: pci: altera: covert to yaml
From: matthew.gerlach @ 2024-04-02 17:14 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: joyce.ooi, bhelgaas, lpieralisi, kw, robh, krzysztof.kozlowski+dt,
conor+dt, linux-pci, devicetree, linux-kernel
In-Reply-To: <20240401224447.GA1762763@bhelgaas>
On Mon, 1 Apr 2024, Bjorn Helgaas wrote:
> "git log --oneline Documentation/devicetree/bindings/pci/" says the
> typical style would be:
>
> dt-bindings: PCI: altera: Convert to YAML
Good suggestion about the 'git log --oneline ...' I will update title in
v2.
>
> On Fri, Mar 29, 2024 at 12:00:31PM -0500, matthew.gerlach@linux.intel.com wrote:
>> From: Matthew Gerlach <matthew.gerlach@linux.intel.com>
>>
>> Covert the device tree bindings for the Altera Root
>> Port controller from text to yaml.
>
> s/covert/convert/ (both in subject and commit log).
>
> Rewrap to fill 80 columns.
>
Thanks for catching the spelling error. Wrapping and spelling fix will be
included in v2.
Matthew Gerlach
^ permalink raw reply
* Re: [RFC PATCH 5/6] watchdog: ROHM BD96801 PMIC WDG driver
From: Guenter Roeck @ 2024-04-02 17:11 UTC (permalink / raw)
To: Matti Vaittinen
Cc: Matti Vaittinen, Lee Jones, Wim Van Sebroeck, devicetree,
linux-kernel, linux-watchdog
In-Reply-To: <f8e743a6c49607de0dd7a27778383477e051b130.1712058690.git.mazziesaccount@gmail.com>
On Tue, Apr 02, 2024 at 04:11:41PM +0300, Matti Vaittinen wrote:
> Introduce driver for WDG block on ROHM BD96801 scalable PMIC.
>
> This driver only supports watchdog with I2C feeding and delayed
> response detection. Whether the watchdog toggles PRSTB pin or
> just causes an interrupt can be configured via device-tree.
>
> The BD96801 PMIC HW supports also window watchdog (too early
> feeding detection) and Q&A mode. These are not supported by
> this driver.
>
> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
> ---
> drivers/watchdog/Kconfig | 13 ++
> drivers/watchdog/Makefile | 1 +
> drivers/watchdog/bd96801_wdt.c | 375 +++++++++++++++++++++++++++++++++
> 3 files changed, 389 insertions(+)
> create mode 100644 drivers/watchdog/bd96801_wdt.c
>
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 6bee137cfbe0..d97e735e1faa 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -181,6 +181,19 @@ config BD957XMUF_WATCHDOG
> watchdog. Alternatively say M to compile the driver as a module,
> which will be called bd9576_wdt.
>
> +config BD96801_WATCHDOG
> + tristate "ROHM BD96801 PMIC Watchdog"
> + depends on MFD_ROHM_BD96801
> + select WATCHDOG_CORE
> + help
> + Support for the watchdog in the ROHM BD96801 PMIC. Watchdog can be
> + configured to only generate IRQ or to trigger system reset via reset
> + pin.
> +
> + Say Y here to include support for the ROHM BD96801 watchdog.
> + Alternatively say M to compile the driver as a module,
> + which will be called bd96801_wdt.
> +
> config CROS_EC_WATCHDOG
> tristate "ChromeOS EC-based watchdog"
> select WATCHDOG_CORE
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index 3710c218f05e..31bc94436c81 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -217,6 +217,7 @@ obj-$(CONFIG_XEN_WDT) += xen_wdt.o
>
> # Architecture Independent
> obj-$(CONFIG_BD957XMUF_WATCHDOG) += bd9576_wdt.o
> +obj-$(CONFIG_BD96801_WATCHDOG) += bd96801_wdt.o
> obj-$(CONFIG_CROS_EC_WATCHDOG) += cros_ec_wdt.o
> obj-$(CONFIG_DA9052_WATCHDOG) += da9052_wdt.o
> obj-$(CONFIG_DA9055_WATCHDOG) += da9055_wdt.o
> diff --git a/drivers/watchdog/bd96801_wdt.c b/drivers/watchdog/bd96801_wdt.c
> new file mode 100644
> index 000000000000..cb2b526ecc21
> --- /dev/null
> +++ b/drivers/watchdog/bd96801_wdt.c
> @@ -0,0 +1,375 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2022 ROHM Semiconductors
> + *
> + * ROHM BD96801 watchdog driver
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/mfd/rohm-bd96801.h>
> +#include <linux/mfd/rohm-generic.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/watchdog.h>
> +
> +static bool nowayout;
> +module_param(nowayout, bool, 0);
> +MODULE_PARM_DESC(nowayout,
> + "Watchdog cannot be stopped once started (default=\"false\")");
> +
> +#define BD96801_WD_TMO_SHORT_MASK 0x70
> +#define BD96801_WD_RATIO_MASK 0x3
> +#define BD96801_WD_TYPE_MASK 0x4
> +#define BD96801_WD_TYPE_SLOW 0x4
> +#define BD96801_WD_TYPE_WIN 0x0
> +
> +#define BD96801_WD_EN_MASK 0x3
> +#define BD96801_WD_IF_EN 0x1
> +#define BD96801_WD_QA_EN 0x2
> +#define BD96801_WD_DISABLE 0x0
> +
> +#define BD96801_WD_ASSERT_MASK 0x8
> +#define BD96801_WD_ASSERT_RST 0x8
> +#define BD96801_WD_ASSERT_IRQ 0x0
> +
> +#define BD96801_WD_FEED_MASK 0x1
> +#define BD96801_WD_FEED 0x1
> +
> +/* units in uS */
> +#define FASTNG_MIN 3370
> +#define BD96801_WDT_DEFAULT_MARGIN 6905120
> +/* Unit is seconds */
> +#define DEFAULT_TIMEOUT 30
> +
> +/*
> + * BD96801 WDG supports window mode so the TMO consists of SHORT and LONG
> + * timeout values. SHORT time is meaningfull only in window mode where feeding
> + * period shorter than SHORT would be an error. LONG time is used to detect if
> + * feeding is not occurring within given time limit (SoC SW hangs). The LONG
> + * timeout time is a multiple of (2, 4, 8 0r 16 times) the SHORT timeout.
> + */
> +
> +struct wdtbd96801 {
> + struct device *dev;
> + struct regmap *regmap;
> + bool always_running;
> + struct watchdog_device wdt;
> +};
> +
> +static int bd96801_wdt_ping(struct watchdog_device *wdt)
> +{
> + struct wdtbd96801 *w = watchdog_get_drvdata(wdt);
> +
> + return regmap_update_bits(w->regmap, BD96801_REG_WD_FEED,
> + BD96801_WD_FEED_MASK, BD96801_WD_FEED);
> +}
> +
> +static int bd96801_wdt_start(struct watchdog_device *wdt)
> +{
> + struct wdtbd96801 *w = watchdog_get_drvdata(wdt);
> + int ret;
> +
> + ret = regmap_update_bits(w->regmap, BD96801_REG_WD_CONF,
> + BD96801_WD_EN_MASK, BD96801_WD_IF_EN);
> +
> + return ret;
> +}
> +
> +static int bd96801_wdt_stop(struct watchdog_device *wdt)
> +{
> + struct wdtbd96801 *w = watchdog_get_drvdata(wdt);
> +
> + if (!w->always_running)
> + return regmap_update_bits(w->regmap, BD96801_REG_WD_CONF,
> + BD96801_WD_EN_MASK, BD96801_WD_DISABLE);
> + set_bit(WDOG_HW_RUNNING, &wdt->status);
> +
> + return 0;
> +}
> +
> +static const struct watchdog_info bd96801_wdt_info = {
> + .options = WDIOF_MAGICCLOSE | WDIOF_KEEPALIVEPING |
> + WDIOF_SETTIMEOUT,
> + .identity = "BD96801 Watchdog",
> +};
> +
> +static const struct watchdog_ops bd96801_wdt_ops = {
> + .start = bd96801_wdt_start,
> + .stop = bd96801_wdt_stop,
> + .ping = bd96801_wdt_ping,
> +};
> +
> +static int find_closest_fast(int target, int *sel, int *val)
> +{
> + int i;
> + int window = FASTNG_MIN;
> +
> + for (i = 0; i < 8 && window < target; i++)
> + window <<= 1;
> +
> + *val = window;
> + *sel = i;
> +
> + if (i == 8)
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +static int find_closest_slow_by_fast(int fast_val, int *target, int *slowsel)
> +{
> + int sel;
> + static const int multipliers[] = {2, 4, 8, 16};
> +
> + for (sel = 0; sel < ARRAY_SIZE(multipliers) &&
> + multipliers[sel] * fast_val < *target; sel++)
> + ;
> +
> + if (sel == ARRAY_SIZE(multipliers))
> + return -EINVAL;
> +
> + *slowsel = sel;
> + *target = multipliers[sel] * fast_val;
> +
> + return 0;
> +}
> +
> +static int find_closest_slow(int *target, int *slow_sel, int *fast_sel)
> +{
> + static const int multipliers[] = {2, 4, 8, 16};
> + int i, j;
> + int val = 0;
> + int window = FASTNG_MIN;
> +
> + for (i = 0; i < 8; i++) {
> + for (j = 0; j < ARRAY_SIZE(multipliers); j++) {
> + int slow;
> +
> + slow = window * multipliers[j];
> + if (slow >= *target && (!val || slow < val)) {
> + val = slow;
> + *fast_sel = i;
> + *slow_sel = j;
> + }
> + }
> + window <<= 1;
> + }
> + if (!val)
> + return -EINVAL;
> +
> + *target = val;
> +
> + return 0;
> +}
> +
> +static int bd96801_set_wdt_mode(struct wdtbd96801 *w, int hw_margin,
> + int hw_margin_min)
> +{
> + int ret, fastng, slowng, type, reg, mask;
> + struct device *dev = w->dev;
> +
> + /* convert to uS */
> + hw_margin *= 1000;
> + hw_margin_min *= 1000;
> + if (hw_margin_min) {
> + int min;
> +
> + type = BD96801_WD_TYPE_WIN;
> + dev_dbg(dev, "Setting type WINDOW 0x%x\n", type);
> + ret = find_closest_fast(hw_margin_min, &fastng, &min);
> + if (ret) {
> + dev_err(dev, "bad WDT window for fast timeout\n");
> + return ret;
> + }
> +
> + ret = find_closest_slow_by_fast(min, &hw_margin, &slowng);
> + if (ret) {
> + dev_err(dev, "bad WDT window\n");
> + return ret;
> + }
> + w->wdt.min_hw_heartbeat_ms = min / 1000;
> + } else {
> + type = BD96801_WD_TYPE_SLOW;
> + dev_dbg(dev, "Setting type SLOW 0x%x\n", type);
> + ret = find_closest_slow(&hw_margin, &slowng, &fastng);
> + if (ret) {
> + dev_err(dev, "bad WDT window\n");
> + return ret;
> + }
> + }
> +
> + w->wdt.max_hw_heartbeat_ms = hw_margin / 1000;
> +
> + fastng <<= ffs(BD96801_WD_TMO_SHORT_MASK) - 1;
> +
> + reg = slowng | fastng;
> + mask = BD96801_WD_RATIO_MASK | BD96801_WD_TMO_SHORT_MASK;
> + ret = regmap_update_bits(w->regmap, BD96801_REG_WD_TMO,
> + mask, reg);
> + if (ret)
> + return ret;
> +
> + ret = regmap_update_bits(w->regmap, BD96801_REG_WD_CONF,
> + BD96801_WD_TYPE_MASK, type);
> +
> + return ret;
> +}
> +
> +static int bd96801_set_heartbeat_from_hw(struct wdtbd96801 *w,
> + unsigned int conf_reg)
> +{
> + int ret;
> + unsigned int val, sel, fast;
> +
> + /*
> + * The BD96801 supports a somewhat peculiar QA-mode, which we do not
> + * support in this driver. If the QA-mode is enabled then we just
> + * warn and bail-out.
> + */
> + if ((conf_reg & BD96801_WD_EN_MASK) != BD96801_WD_IF_EN) {
> + dev_warn(w->dev, "watchdog set to Q&A mode - exiting\n");
> + return -EINVAL;
> + }
> +
> + ret = regmap_read(w->regmap, BD96801_REG_WD_TMO, &val);
> + if (ret)
> + return ret;
> +
> + sel = val & BD96801_WD_TMO_SHORT_MASK;
> + sel >>= ffs(BD96801_WD_TMO_SHORT_MASK) - 1;
> + fast = FASTNG_MIN << sel;
> +
> + sel = (val & BD96801_WD_RATIO_MASK) + 1;
> + w->wdt.max_hw_heartbeat_ms = (fast << sel) / USEC_PER_MSEC;
> +
> + if ((conf_reg & BD96801_WD_TYPE_MASK) == BD96801_WD_TYPE_WIN)
> + w->wdt.min_hw_heartbeat_ms = fast / USEC_PER_MSEC;
> +
> + return 0;
> +}
> +
> +static int init_wdg_hw(struct wdtbd96801 *w)
> +{
> + u32 hw_margin[2];
> + int count, ret;
> + u32 hw_margin_max = BD96801_WDT_DEFAULT_MARGIN, hw_margin_min = 0;
> +
> + count = device_property_count_u32(w->dev->parent, "rohm,hw-timeout-ms");
> + if (count < 0 && count != -EINVAL)
> + return count;
> +
> + if (count > 0) {
> + if (count > ARRAY_SIZE(hw_margin))
> + return -EINVAL;
> +
> + ret = device_property_read_u32_array(w->dev->parent,
> + "rohm,hw-timeout-ms",
> + &hw_margin[0], count);
> + if (ret < 0)
> + return ret;
> +
> + if (count == 1)
> + hw_margin_max = hw_margin[0];
> +
> + if (count == 2) {
> + hw_margin_max = hw_margin[1];
> + hw_margin_min = hw_margin[0];
> + }
> + }
> +
> + ret = bd96801_set_wdt_mode(w, hw_margin_max, hw_margin_min);
> + if (ret)
> + return ret;
> +
> + ret = device_property_match_string(w->dev->parent, "rohm,wdg-action",
> + "prstb");
> + if (ret >= 0) {
> + ret = regmap_update_bits(w->regmap, BD96801_REG_WD_CONF,
> + BD96801_WD_ASSERT_MASK,
> + BD96801_WD_ASSERT_RST);
> + return ret;
> + }
> +
> + ret = device_property_match_string(w->dev->parent, "rohm,wdg-action",
> + "intb-only");
> + if (ret >= 0) {
> + ret = regmap_update_bits(w->regmap, BD96801_REG_WD_CONF,
> + BD96801_WD_ASSERT_MASK,
> + BD96801_WD_ASSERT_IRQ);
> + return ret;
> + }
I don't see the devicetree bindings documented in the series.
I am also a bit surprised that the interrupt isn't handled in the driver.
Please explain.
> +
> + return 0;
> +}
> +
> +static int bd96801_wdt_probe(struct platform_device *pdev)
> +{
> + struct wdtbd96801 *w;
> + int ret;
> + unsigned int val;
> +
> + w = devm_kzalloc(&pdev->dev, sizeof(*w), GFP_KERNEL);
> + if (!w)
> + return -ENOMEM;
> +
> + w->regmap = dev_get_regmap(pdev->dev.parent, NULL);
dev_get_regmap() can return NULL.
> + w->dev = &pdev->dev;
> +
> + w->wdt.info = &bd96801_wdt_info;
> + w->wdt.ops = &bd96801_wdt_ops;
> + w->wdt.parent = pdev->dev.parent;
> + w->wdt.timeout = DEFAULT_TIMEOUT;
> + watchdog_set_drvdata(&w->wdt, w);
> +
> + w->always_running = device_property_read_bool(pdev->dev.parent,
> + "always-running");
> +
Without documentation, it looks like the always-running (from
linux,wdt-gpio.yaml) may be abused. Its defined meaning is
"the watchdog is always running and can not be stopped". Its
use here seems to be "start watchdog when loading the module and
prevent it from being stopped".
Oh well, looks like the abuse was introduced with bd9576_wdt. That
doesn't make it better. At the very least it needs to be documented
that the property does not have the intended (documented) meaning.
> + ret = regmap_read(w->regmap, BD96801_REG_WD_CONF, &val);
> + if (ret)
> + return dev_err_probe(&pdev->dev, ret,
> + "Failed to get the watchdog state\n");
> +
> + /*
> + * If the WDG is already enabled we assume it is configured by boot.
> + * In this case we just update the hw-timeout based on values set to
> + * the timeout / mode registers and leave the hardware configs
> + * untouched.
> + */
> + if ((val & BD96801_WD_EN_MASK) != BD96801_WD_DISABLE) {
> + dev_dbg(&pdev->dev, "watchdog was running during probe\n");
> + ret = bd96801_set_heartbeat_from_hw(w, val);
> + if (ret)
> + return ret;
> +
> + set_bit(WDOG_HW_RUNNING, &w->wdt.status);
> + } else {
> + /* If WDG is not running so we will initializate it */
> + ret = init_wdg_hw(w);
> + if (ret)
> + return ret;
> + }
> +
> + watchdog_init_timeout(&w->wdt, 0, pdev->dev.parent);
> + watchdog_set_nowayout(&w->wdt, nowayout);
> + watchdog_stop_on_reboot(&w->wdt);
> +
> + if (w->always_running)
> + bd96801_wdt_start(&w->wdt);
I think this needs to set WDOG_HW_RUNNING or the watchdog will trigger
a reboot if the watchdog device is not opened and the watchdog wasn't
already running when the module was loaded.
That makes me wonder what happens if the property is set and the
watchdog daemon isn't started in the bd9576_wdt driver.
> +
> + return devm_watchdog_register_device(&pdev->dev, &w->wdt);
> +}
> +
> +static struct platform_driver bd96801_wdt = {
> + .driver = {
> + .name = "bd96801-wdt"
> + },
> + .probe = bd96801_wdt_probe,
> +};
> +module_platform_driver(bd96801_wdt);
> +
> +MODULE_AUTHOR("Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>");
> +MODULE_DESCRIPTION("BD96801 watchdog driver");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:bd96801-wdt");
> --
> 2.43.2
>
>
> --
> Matti Vaittinen, Linux device drivers
> ROHM Semiconductors, Finland SWDC
> Kiviharjunlenkki 1E
> 90220 OULU
> FINLAND
>
> ~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
> Simon says - in Latin please.
> ~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
> Thanks to Simon Glass for the translation =]
^ permalink raw reply
* Re: [PATCH] arm64: dts: ti: k3-j722s: Disable ethernet ports by default
From: Francesco Dolcini @ 2024-04-02 16:58 UTC (permalink / raw)
To: Michael Walle
Cc: Nishanth Menon, Vignesh Raghavendra, Tero Kristo, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-arm-kernel, devicetree,
linux-kernel
In-Reply-To: <20240402151802.3803708-1-mwalle@kernel.org>
Hello Michael,
On Tue, Apr 02, 2024 at 05:18:02PM +0200, Michael Walle wrote:
> Device tree best practice is to disable any external interface in the
> dtsi and just enable them if needed in the device tree. Thus, disable
> both ethernet ports by default and just enable the one used by the EVM
> in its device tree.
>
> There is no functional change.
>
> Signed-off-by: Michael Walle <mwalle@kernel.org>
> ---
> This should also be true for all the other SoCs. But I don't wanted to
> touch all the (older) device trees. j722s is pretty new, so there we
> should get it right.
> ---
> arch/arm64/boot/dts/ti/k3-j722s-evm.dts | 5 +----
> arch/arm64/boot/dts/ti/k3-j722s.dtsi | 8 ++++++++
> 2 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/ti/k3-j722s-evm.dts b/arch/arm64/boot/dts/ti/k3-j722s-evm.dts
> index d045dc7dde0c..afe7f68e6a4b 100644
> --- a/arch/arm64/boot/dts/ti/k3-j722s-evm.dts
> +++ b/arch/arm64/boot/dts/ti/k3-j722s-evm.dts
> @@ -224,14 +224,11 @@ cpsw3g_phy0: ethernet-phy@0 {
> };
>
> &cpsw_port1 {
> + status = "okay";
status should be the last property, according to the dts coding guidelines.
> phy-mode = "rgmii-rxid";
> phy-handle = <&cpsw3g_phy0>;
> };
Francesco
^ permalink raw reply
* Re: [PATCH 1/2] drm/bridge: lt8912b: add support for P/N pin swap
From: Francesco Dolcini @ 2024-04-02 16:53 UTC (permalink / raw)
To: Alexandru Ardelean
Cc: linux-kernel, dri-devel, devicetree, adrien.grassein,
andrzej.hajda, neil.armstrong, rfoss, Laurent.pinchart, jonas,
jernej.skrabec, airlied, daniel, maarten.lankhorst, mripard,
tzimmermann, robh, krzysztof.kozlowski+dt, conor+dt,
stefan.eichenberger, francesco.dolcini, marius.muresan,
irina.muresan
In-Reply-To: <20240402105925.905144-1-alex@shruggie.ro>
Hello Alexandru, thanks for your patch.
On Tue, Apr 02, 2024 at 01:59:24PM +0300, Alexandru Ardelean wrote:
> On some HW designs, it's easier for the layout if the P/N pins are swapped.
> In those cases, we need to adjust (for this) by configuring the MIPI analog
> registers differently. Specifically, register 0x3e needs to be 0xf6
> (instead of 0xd6).
>
> This change adds a 'lontium,pn-swap' device-tree property to configure the
> MIPI analog registers for P/N swap.
>
> Signed-off-by: Alexandru Ardelean <alex@shruggie.ro>
> ---
> drivers/gpu/drm/bridge/lontium-lt8912b.c | 25 +++++++++++++++++++++---
> 1 file changed, 22 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/lontium-lt8912b.c b/drivers/gpu/drm/bridge/lontium-lt8912b.c
> index 4b2ae27f0a57f..154126bb922b4 100644
> --- a/drivers/gpu/drm/bridge/lontium-lt8912b.c
> +++ b/drivers/gpu/drm/bridge/lontium-lt8912b.c
> @@ -47,6 +47,7 @@ struct lt8912 {
>
> u8 data_lanes;
> bool is_power_on;
> + bool do_pn_swap;
> };
>
> static int lt8912_write_init_config(struct lt8912 *lt)
> @@ -78,15 +79,31 @@ static int lt8912_write_init_config(struct lt8912 *lt)
> {0x55, 0x44},
> {0x57, 0x01},
> {0x5a, 0x02},
> -
> - /*MIPI Analog*/
> + };
> + const struct reg_sequence mipi_analog_seq[] = {
> {0x3e, 0xd6},
> {0x3f, 0xd4},
> {0x41, 0x3c},
> {0xB2, 0x00},
> };
> + const struct reg_sequence mipi_analog_pn_swap_seq[] = {
> + {0x3e, 0xf6},
> + {0x3f, 0xd4},
> + {0x41, 0x3c},
> + {0xB2, 0x00},
> + };
> + int ret;
>
> - return regmap_multi_reg_write(lt->regmap[I2C_MAIN], seq, ARRAY_SIZE(seq));
> + ret = regmap_multi_reg_write(lt->regmap[I2C_MAIN], seq, ARRAY_SIZE(seq));
> + if (ret < 0)
> + return ret;
> +
> + if (!lt->do_pn_swap)
> + return regmap_multi_reg_write(lt->regmap[I2C_MAIN], mipi_analog_seq,
> + ARRAY_SIZE(mipi_analog_seq));
> +
> + return regmap_multi_reg_write(lt->regmap[I2C_MAIN], mipi_analog_pn_swap_seq,
> + ARRAY_SIZE(mipi_analog_pn_swap_seq));
Can you just remove {0x3e, 0xd6} from the register/value array and write
it afterward depending on `do_pn_swap` value? Or keep it with the
current value and only overwrite it when do_pn_swap is true?
If you do it this way is a 4 line change.
> static int lt8912_write_mipi_basic_config(struct lt8912 *lt)
> @@ -702,6 +719,8 @@ static int lt8912_parse_dt(struct lt8912 *lt)
> }
> lt->gp_reset = gp_reset;
>
> + lt->do_pn_swap = device_property_read_bool(dev, "lontium,pn-swap");
I would call this variable the same that is called in the lontium
documentation, mipirx_diff_swap
Francesco
^ permalink raw reply
* Re: [PATCH v6 3/4] firmware: arm_scmi: Add SCMI v3.2 pincontrol protocol basic support
From: Andy Shevchenko @ 2024-04-02 16:39 UTC (permalink / raw)
To: Cristian Marussi
Cc: Peng Fan, Peng Fan (OSS), Sudeep Holla, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Linus Walleij, Dan Carpenter,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
linux-gpio@vger.kernel.org, Oleksii Moisieiev
In-Reply-To: <ZgwrKnx3hb59OG77@pluto>
On Tue, Apr 2, 2024 at 6:58 PM Cristian Marussi
<cristian.marussi@arm.com> wrote:
> On Tue, Apr 02, 2024 at 04:06:06PM +0300, Andy Shevchenko wrote:
> > On Tue, Apr 2, 2024 at 10:48 AM Cristian Marussi
> > <cristian.marussi@arm.com> wrote:
> > > On Sun, Mar 31, 2024 at 01:44:28PM +0000, Peng Fan wrote:
> > > > > Sat, Mar 23, 2024 at 08:15:16PM +0800, Peng Fan (OSS) kirjoitti:
...
> > > > > > +#include <linux/module.h>
> > > > > > +#include <linux/scmi_protocol.h>
> > > > > > +#include <linux/slab.h>
> > > > >
> > > > > This is semi-random list of headers. Please, follow IWYU principle (include
> > > > > what you use). There are a lot of inclusions I see missing (just in the context of
> > > > > this page I see bits.h, types.h, and asm/byteorder.h).
> > > >
> > > > Is there any documentation about this requirement?
> > > > Some headers are already included by others.
> >
> > The documentation here is called "a common sense".
> > The C language is built like this and we expect that nobody will
> > invest into the dependency hell that we have already, that's why IWYU
> > principle, please follow it.
>
> Yes, but given that we have a growing number of SCMI protocols there is a
> common local protocols.h header to group all includes needed by any
> protocols: the idea behind this (and the devm_ saga down below) was to ease
> development of protocols, since there are lots of them and growing, given
> the SCMI spec is extensible.
Yes, and what you are effectively suggesting is: "Technical debt? Oh,
fine, we do not care!" This is not good. I'm in a long term of
cleaning up the dependency hell in the kernel (my main focus is
kernel.h for now) and I am talking from my experience. I do not like
what people are doing in 95% of the code, that's why I really want to
stop the bad practices as soon as possible.
Last to add, but not least is that your code may be used as an example
for others, hence we really have to do our best in order to avoid bad
design, practices, and cargo cults. If this requires more refactoring
of the existing code, then do it sooner than later.
...
> > > Andy made (mostly) the same remarks on this same patch ~1-year ago on
> > > this same patch while it was posted by Oleksii.
> > >
> > > And I told that time that most of the remarks around devm_ usage were
> > > wrong due to how the SCMI core handles protocol initialization (using a
> > > devres group transparently).
> > >
> > > This is what I answered that time.
> > >
> > > https://lore.kernel.org/linux-arm-kernel/ZJ78hBcjAhiU+ZBO@e120937-lin/#t
> > >
> > > I wont repeat myself, but, in a nutshell the memory allocation like it
> > > is now is fine: a bit happens via devm_ at protocol initialization, the
> > > other is doe via explicit kmalloc at runtime and freed via kfree at
> > > remove time (if needed...i.e. checking the present flag of some structs)
> >
> > This sounds like a mess. devm_ is expected to be used only for the
> > ->probe() stage, otherwise you may consider cleanup.h (__free() macro)
> > to have automatic free at the paths where memory is not needed.
>
> Indeed, this protocol_init code is called by the SCMI core once for all when
> an SCMI driver tries at first to use this specific protocol by 'getting' its
> protocol_ops, so it is indeed called inside the probe chain of the driver:
> at this point you *can* decide to use devres to allocate memory and be assured
> that if the init fails, or when the driver cease to use this protocol (calling
> its remove()) and no other driver is using it, all the stuff that have been
> allocated related to this protocol will be released by the core for you.
> (using an internal devres group)
>
> Without this you should handle manually all the deallocation manually on
> the init error-paths AND also provide all the cleanup explicitly when
> the protocol is no more used by any driver (multiple users of the same
> protocol instance are possible)...for all protocols.
Yes. Is it a problem?
> This is/was handy since, till now, all the SCMI querying and resources
> allocation happened anyway all at once at init time...
>
> ...the mess, as you kindly called it, derives from the fact that this specific
> protocol is the first and only one that does NOT allocate all that it needs
> during the initialization (to minimize needless allocs for a lot of possibly
> unused resources) and this lazy-initialization phase, done after init at runtime,
> must be handled manually since it cannot be managed by the devres group that is
> open/clsoed around init by the SCMI core.
>
> I dont like particularly this split allocation but it has a reason and any
> other solution seems more messy to me at the moment.
>
> And I dont feel like changing all the SCMI protocol initialziation core code
> (that address a lot more under the hood) is a desirable solution to address a
> non-existent problem really.
>
> > And the function naming doesn't suggest that you have a probe-remove
> > pair. Moreover, if the init-deinit part is called in the probe-remove,
> > the devm_ must not be mixed with non-devm ones, as it breaks the order
> > and leads to subtle mistakes.
>
> Initialization order is enforced by SCMI core like this:
>
> @driver_probe->get_protocol_ops()
> @core/get_protocol_ops
> -> devres_group_open()
> -> protocol_init->devm_*()
> -> devres_group_close()
> -> driver_probing
>
> @runtime optional explicit_lazy_kmallocs inside the protocol
>
> @driver_remove->put_protocol_ops()
> @core/put_protocol_ops()
> -> protocol_denit->optional_explicit_kfree_of_the_above
> -> devres_group_release()
> -> driver_removing
>
> ... dont think there's an ordering problem.
The mess with devm_ vs. non-devm is quite easy to achieve. You are
probably out of the control of what the protocol driver wants to do in
the init. Is the usage of devm (which APIs and in which order can be
used) WRT SCMI documented somewhere?
Misuse of devm is a common issue, I'm not surprised it will hit your
subsystem one day with such an approach.
> ...note that the ph->dev provided in the protocol_init and used by devm_
> is NOT the dev of the SCMI driver probe/remove that uses the get_protocol_ops,
> it is an internal SCMI device associated with the core SCMI stack probing and
> allocations, within which a devres group for the specific protocol is created
> when that specific protocol is initialized...protocols are not fully
> fledged drivers are just bits of the SCMI stack that are initialized when needed
> (and possibly also loaded when needed for vendor protocols) and
> de-initialzed when no more SCMI driver users exist for that protocol.
P.S. I guess from now on it's your call, but this code and in case
other drivers use similar, is badly written. I hope some documentation
exists to at least justify all this mess and explaining why there no
and (what's really important) will never be a problem.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v7 4/4] pinctrl: Implementation of the generic scmi-pinctrl driver
From: Cristian Marussi @ 2024-04-02 16:40 UTC (permalink / raw)
To: Dan Carpenter
Cc: Andy Shevchenko, Peng Fan (OSS), Sudeep Holla, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Linus Walleij,
linux-arm-kernel, linux-kernel, devicetree, linux-gpio, Peng Fan,
Oleksii Moisieiev
In-Reply-To: <c5bdf039-c43b-4611-9f0b-81585e296206@moroto.mountain>
On Tue, Apr 02, 2024 at 05:09:34PM +0300, Dan Carpenter wrote:
> On Tue, Apr 02, 2024 at 04:22:45PM +0300, Andy Shevchenko wrote:
> > On Tue, Apr 02, 2024 at 10:22:24AM +0800, Peng Fan (OSS) wrote:
> > > +static int pinctrl_scmi_get_pins(struct scmi_pinctrl *pmx,
> > > + struct pinctrl_desc *desc)
> > > +{
> > > + struct pinctrl_pin_desc *pins;
> > > + unsigned int npins;
> > > + int ret, i;
> > > +
> > > + npins = pinctrl_ops->count_get(pmx->ph, PIN_TYPE);
> > > + /*
> > > + * npins will never be zero, the scmi pinctrl driver has bailed out
> > > + * if npins is zero.
> > > + */
> >
> > This is fragile, but at least it is documented.
> >
>
> It was never clear to me where the crash would happen if npins was zero.
> Does some part of pinctrl internals assume we have at least one pin?
Dont think there were any possible crashes since at the protoocl layer
(not here) kcalloc returns ZERO_SIZE_PTR into pinfo->pins for a zero-bytes
allocation BUT it is indeed never accessed since any attempt to access a
pin will be considerd invalid (any u32 index >= (nr_pins=0))...
...but what is the point of loading protocol and drivers with zero pins ?
You can have zero grouos and zero functions, but zero pins ?
Thanks,
Cristian
^ permalink raw reply
* Re: [PATCH v2 12/15] dt-bindings: thermal: mediatek: Add LVTS thermal controller definition for MT8188
From: Rob Herring @ 2024-04-02 16:23 UTC (permalink / raw)
To: Nicolas Pitre
Cc: Nicolas Pitre, Daniel Lezcano, AngeloGioacchino Del Regno,
devicetree, linux-pm, linux-mediatek
In-Reply-To: <20240402032729.2736685-13-nico@fluxnic.net>
On Mon, 01 Apr 2024 23:25:46 -0400, Nicolas Pitre wrote:
> From: Nicolas Pitre <npitre@baylibre.com>
>
> Add LVTS thermal controller definition for MT8188.
>
> Signed-off-by: Nicolas Pitre <npitre@baylibre.com>
> ---
> .../bindings/thermal/mediatek,lvts-thermal.yaml | 4 ++++
> .../dt-bindings/thermal/mediatek,lvts-thermal.h | 16 ++++++++++++++++
> 2 files changed, 20 insertions(+)
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v2 1/3] dt-bindings: leds: Add Silergy SY7802 flash LED
From: Rob Herring @ 2024-04-02 16:22 UTC (permalink / raw)
To: André Apitzsch
Cc: phone-devel, devicetree, linux-hardening, linux-leds,
~postmarketos/upstreaming, Gustavo A. R. Silva, Konrad Dybcio,
Lee Jones, Conor Dooley, Krzysztof Kozlowski, Bjorn Andersson,
linux-kernel, linux-arm-msm, Kees Cook, Pavel Machek
In-Reply-To: <20240401-sy7802-v2-1-1138190a7448@apitzsch.eu>
On Mon, 01 Apr 2024 23:23:55 +0200, André Apitzsch wrote:
> Document Silergy SY7802 flash LED driver devicetree bindings.
>
> Signed-off-by: André Apitzsch <git@apitzsch.eu>
> ---
> .../devicetree/bindings/leds/silergy,sy7802.yaml | 100 +++++++++++++++++++++
> 1 file changed, 100 insertions(+)
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH] arm64: dts: ti: k3-j722s: Disable ethernet ports by default
From: Nishanth Menon @ 2024-04-02 16:19 UTC (permalink / raw)
To: Michael Walle
Cc: Vignesh Raghavendra, Tero Kristo, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-arm-kernel, devicetree,
linux-kernel
In-Reply-To: <D09RSRPKV9L9.2TS01DA84TEKK@kernel.org>
On 18:16-20240402, Michael Walle wrote:
> Hi,
>
> > This is better at arch/arm64/boot/dts/ti/k3-am62p-main.dtsi.
>
> I'm fine with that, but be aware that we'll also change the am62p
> SoC dtsi in that case.
This change is valid to me (sigh.. it's been a whack-a-mole as you can expect).
62p and j722s/am67 are too closely related anyways.. but, this will need
to percolate consistently to all k3 SoCs.. (different problem).
--
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D
^ permalink raw reply
* Re: [PATCH v1 6/6] clk: meson: a1: add Amlogic A1 CPU clock controller driver
From: Jerome Brunet @ 2024-04-02 16:17 UTC (permalink / raw)
To: Dmitry Rokosov
Cc: Jerome Brunet, neil.armstrong, mturquette, sboyd, robh+dt,
krzysztof.kozlowski+dt, khilman, martin.blumenstingl, kernel,
rockosov, linux-amlogic, linux-clk, devicetree, linux-kernel,
linux-arm-kernel
In-Reply-To: <20240402161110.v7t72s6t7m3bzifi@CAB-WSD-L081021>
On Tue 02 Apr 2024 at 19:11, Dmitry Rokosov <ddrokosov@salutedevices.com> wrote:
> On Tue, Apr 02, 2024 at 04:11:12PM +0200, Jerome Brunet wrote:
>>
>> On Tue 02 Apr 2024 at 14:05, Dmitry Rokosov <ddrokosov@salutedevices.com> wrote:
>>
>> > Hello Jerome,
>> >
>> > On Tue, Apr 02, 2024 at 11:35:49AM +0200, Jerome Brunet wrote:
>> >>
>> >> On Fri 29 Mar 2024 at 23:58, Dmitry Rokosov <ddrokosov@salutedevices.com> wrote:
>> >>
>> >> > The CPU clock controller plays a general role in the Amlogic A1 SoC
>> >> > family by generating CPU clocks. As an APB slave module, it offers the
>> >> > capability to inherit the CPU clock from two sources: the internal fixed
>> >> > clock known as 'cpu fixed clock' and the external input provided by the
>> >> > A1 PLL clock controller, referred to as 'syspll'.
>> >> >
>> >> > It is important for the driver to handle cpu_clk rate switching
>> >> > effectively by transitioning to the CPU fixed clock to avoid any
>> >> > potential execution freezes.
>> >> >
>> >> > Signed-off-by: Dmitry Rokosov <ddrokosov@salutedevices.com>
>> >> > ---
>> >> > drivers/clk/meson/Kconfig | 10 ++
>> >> > drivers/clk/meson/Makefile | 1 +
>> >> > drivers/clk/meson/a1-cpu.c | 324 +++++++++++++++++++++++++++++++++++++
>> >> > drivers/clk/meson/a1-cpu.h | 16 ++
>> >> > 4 files changed, 351 insertions(+)
>> >> > create mode 100644 drivers/clk/meson/a1-cpu.c
>> >> > create mode 100644 drivers/clk/meson/a1-cpu.h
>> >> >
>> >> > diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
>> >> > index 80c4a18c83d2..148d4495eee3 100644
>> >> > --- a/drivers/clk/meson/Kconfig
>> >> > +++ b/drivers/clk/meson/Kconfig
>> >> > @@ -111,6 +111,16 @@ config COMMON_CLK_AXG_AUDIO
>> >> > Support for the audio clock controller on AmLogic A113D devices,
>> >> > aka axg, Say Y if you want audio subsystem to work.
>> >> >
>> >> > +config COMMON_CLK_A1_CPU
>> >> > + tristate "Amlogic A1 SoC CPU controller support"
>> >> > + depends on ARM64
>> >> > + select COMMON_CLK_MESON_REGMAP
>> >> > + select COMMON_CLK_MESON_CLKC_UTILS
>> >> > + help
>> >> > + Support for the CPU clock controller on Amlogic A113L based
>> >> > + device, A1 SoC Family. Say Y if you want A1 CPU clock controller
>> >> > + to work.
>> >> > +
>> >> > config COMMON_CLK_A1_PLL
>> >> > tristate "Amlogic A1 SoC PLL controller support"
>> >> > depends on ARM64
>> >> > diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
>> >> > index 4968fc7ad555..2a06eb0303d6 100644
>> >> > --- a/drivers/clk/meson/Makefile
>> >> > +++ b/drivers/clk/meson/Makefile
>> >> > @@ -18,6 +18,7 @@ obj-$(CONFIG_COMMON_CLK_MESON_AUDIO_RSTC) += meson-audio-rstc.o
>> >> >
>> >> > obj-$(CONFIG_COMMON_CLK_AXG) += axg.o axg-aoclk.o
>> >> > obj-$(CONFIG_COMMON_CLK_AXG_AUDIO) += axg-audio.o
>> >> > +obj-$(CONFIG_COMMON_CLK_A1_CPU) += a1-cpu.o
>> >> > obj-$(CONFIG_COMMON_CLK_A1_PLL) += a1-pll.o
>> >> > obj-$(CONFIG_COMMON_CLK_A1_PERIPHERALS) += a1-peripherals.o
>> >> > obj-$(CONFIG_COMMON_CLK_A1_AUDIO) += a1-audio.o
>> >> > diff --git a/drivers/clk/meson/a1-cpu.c b/drivers/clk/meson/a1-cpu.c
>> >> > new file mode 100644
>> >> > index 000000000000..5f5d8ae112e5
>> >> > --- /dev/null
>> >> > +++ b/drivers/clk/meson/a1-cpu.c
>> >> > @@ -0,0 +1,324 @@
>> >> > +// SPDX-License-Identifier: GPL-2.0+
>> >> > +/*
>> >> > + * Amlogic A1 SoC family CPU Clock Controller driver.
>> >> > + *
>> >> > + * Copyright (c) 2024, SaluteDevices. All Rights Reserved.
>> >> > + * Author: Dmitry Rokosov <ddrokosov@salutedevices.com>
>> >> > + */
>> >> > +
>> >> > +#include <linux/clk.h>
>> >> > +#include <linux/clk-provider.h>
>> >> > +#include <linux/mod_devicetable.h>
>> >> > +#include <linux/platform_device.h>
>> >> > +#include "a1-cpu.h"
>> >> > +#include "clk-regmap.h"
>> >> > +#include "meson-clkc-utils.h"
>> >> > +
>> >> > +#include <dt-bindings/clock/amlogic,a1-cpu-clkc.h>
>> >> > +
>> >> > +static u32 cpu_fsource_sel_table[] = { 0, 1, 2 };
>> >> > +static const struct clk_parent_data cpu_fsource_sel_parents[] = {
>> >> > + { .fw_name = "xtal" },
>> >> > + { .fw_name = "fclk_div2" },
>> >> > + { .fw_name = "fclk_div3" },
>> >> > +};
>> >> > +
>> >> > +static struct clk_regmap cpu_fsource_sel0 = {
>> >> > + .data = &(struct clk_regmap_mux_data) {
>> >> > + .offset = CPUCTRL_CLK_CTRL0,
>> >> > + .mask = 0x3,
>> >> > + .shift = 0,
>> >> > + .table = cpu_fsource_sel_table,
>> >> > + },
>> >> > + .hw.init = &(struct clk_init_data) {
>> >> > + .name = "cpu_fsource_sel0",
>> >> > + .ops = &clk_regmap_mux_ops,
>> >> > + .parent_data = cpu_fsource_sel_parents,
>> >> > + .num_parents = ARRAY_SIZE(cpu_fsource_sel_parents),
>> >> > + .flags = CLK_SET_RATE_PARENT,
>> >> > + },
>> >> > +};
>> >> > +
>> >> > +static struct clk_regmap cpu_fsource_div0 = {
>> >> > + .data = &(struct clk_regmap_div_data) {
>> >> > + .offset = CPUCTRL_CLK_CTRL0,
>> >> > + .shift = 4,
>> >> > + .width = 6,
>> >> > + },
>> >> > + .hw.init = &(struct clk_init_data) {
>> >> > + .name = "cpu_fsource_div0",
>> >> > + .ops = &clk_regmap_divider_ops,
>> >> > + .parent_hws = (const struct clk_hw *[]) {
>> >> > + &cpu_fsource_sel0.hw
>> >> > + },
>> >> > + .num_parents = 1,
>> >> > + .flags = CLK_SET_RATE_PARENT,
>> >> > + },
>> >> > +};
>> >> > +
>> >> > +static struct clk_regmap cpu_fsel0 = {
>> >> > + .data = &(struct clk_regmap_mux_data) {
>> >> > + .offset = CPUCTRL_CLK_CTRL0,
>> >> > + .mask = 0x1,
>> >> > + .shift = 2,
>> >> > + },
>> >> > + .hw.init = &(struct clk_init_data) {
>> >> > + .name = "cpu_fsel0",
>> >> > + .ops = &clk_regmap_mux_ops,
>> >> > + .parent_hws = (const struct clk_hw *[]) {
>> >> > + &cpu_fsource_sel0.hw,
>> >> > + &cpu_fsource_div0.hw,
>> >> > + },
>> >> > + .num_parents = 2,
>> >> > + .flags = CLK_SET_RATE_PARENT,
>> >> > + },
>> >> > +};
>> >> > +
>> >> > +static struct clk_regmap cpu_fsource_sel1 = {
>> >> > + .data = &(struct clk_regmap_mux_data) {
>> >> > + .offset = CPUCTRL_CLK_CTRL0,
>> >> > + .mask = 0x3,
>> >> > + .shift = 16,
>> >> > + .table = cpu_fsource_sel_table,
>> >> > + },
>> >> > + .hw.init = &(struct clk_init_data) {
>> >> > + .name = "cpu_fsource_sel1",
>> >> > + .ops = &clk_regmap_mux_ops,
>> >> > + .parent_data = cpu_fsource_sel_parents,
>> >> > + .num_parents = ARRAY_SIZE(cpu_fsource_sel_parents),
>> >> > + .flags = CLK_SET_RATE_PARENT,
>> >> > + },
>> >> > +};
>> >> > +
>> >> > +static struct clk_regmap cpu_fsource_div1 = {
>> >> > + .data = &(struct clk_regmap_div_data) {
>> >> > + .offset = CPUCTRL_CLK_CTRL0,
>> >> > + .shift = 20,
>> >> > + .width = 6,
>> >> > + },
>> >> > + .hw.init = &(struct clk_init_data) {
>> >> > + .name = "cpu_fsource_div1",
>> >> > + .ops = &clk_regmap_divider_ops,
>> >> > + .parent_hws = (const struct clk_hw *[]) {
>> >> > + &cpu_fsource_sel1.hw
>> >> > + },
>> >> > + .num_parents = 1,
>> >> > + .flags = CLK_SET_RATE_PARENT,
>> >> > + },
>> >> > +};
>> >> > +
>> >> > +static struct clk_regmap cpu_fsel1 = {
>> >> > + .data = &(struct clk_regmap_mux_data) {
>> >> > + .offset = CPUCTRL_CLK_CTRL0,
>> >> > + .mask = 0x1,
>> >> > + .shift = 18,
>> >> > + },
>> >> > + .hw.init = &(struct clk_init_data) {
>> >> > + .name = "cpu_fsel1",
>> >> > + .ops = &clk_regmap_mux_ops,
>> >> > + .parent_hws = (const struct clk_hw *[]) {
>> >> > + &cpu_fsource_sel1.hw,
>> >> > + &cpu_fsource_div1.hw,
>> >> > + },
>> >> > + .num_parents = 2,
>> >> > + .flags = CLK_SET_RATE_PARENT,
>> >> > + },
>> >> > +};
>> >> > +
>> >> > +static struct clk_regmap cpu_fclk = {
>> >> > + .data = &(struct clk_regmap_mux_data) {
>> >> > + .offset = CPUCTRL_CLK_CTRL0,
>> >> > + .mask = 0x1,
>> >> > + .shift = 10,
>> >> > + },
>> >> > + .hw.init = &(struct clk_init_data) {
>> >> > + .name = "cpu_fclk",
>> >> > + .ops = &clk_regmap_mux_ops,
>> >> > + .parent_hws = (const struct clk_hw *[]) {
>> >> > + &cpu_fsel0.hw,
>> >> > + &cpu_fsel1.hw,
>> >> > + },
>> >> > + .num_parents = 2,
>> >> > + .flags = CLK_SET_RATE_PARENT,
>> >> > + },
>> >> > +};
>> >> > +
>> >> > +static struct clk_regmap cpu_clk = {
>> >> > + .data = &(struct clk_regmap_mux_data) {
>> >> > + .offset = CPUCTRL_CLK_CTRL0,
>> >> > + .mask = 0x1,
>> >> > + .shift = 11,
>> >> > + },
>> >> > + .hw.init = &(struct clk_init_data) {
>> >> > + .name = "cpu_clk",
>> >> > + .ops = &clk_regmap_mux_ops,
>> >> > + .parent_data = (const struct clk_parent_data []) {
>> >> > + { .hw = &cpu_fclk.hw },
>> >> > + { .fw_name = "sys_pll", },
>> >> > + },
>> >> > + .num_parents = 2,
>> >> > + .flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
>> >> > + },
>> >> > +};
>> >> > +
>> >> > +/* Array of all clocks registered by this provider */
>> >> > +static struct clk_hw *a1_cpu_hw_clks[] = {
>> >> > + [CLKID_CPU_FSOURCE_SEL0] = &cpu_fsource_sel0.hw,
>> >> > + [CLKID_CPU_FSOURCE_DIV0] = &cpu_fsource_div0.hw,
>> >> > + [CLKID_CPU_FSEL0] = &cpu_fsel0.hw,
>> >> > + [CLKID_CPU_FSOURCE_SEL1] = &cpu_fsource_sel1.hw,
>> >> > + [CLKID_CPU_FSOURCE_DIV1] = &cpu_fsource_div1.hw,
>> >> > + [CLKID_CPU_FSEL1] = &cpu_fsel1.hw,
>> >> > + [CLKID_CPU_FCLK] = &cpu_fclk.hw,
>> >> > + [CLKID_CPU_CLK] = &cpu_clk.hw,
>> >> > +};
>> >> > +
>> >> > +static struct clk_regmap *const a1_cpu_regmaps[] = {
>> >> > + &cpu_fsource_sel0,
>> >> > + &cpu_fsource_div0,
>> >> > + &cpu_fsel0,
>> >> > + &cpu_fsource_sel1,
>> >> > + &cpu_fsource_div1,
>> >> > + &cpu_fsel1,
>> >> > + &cpu_fclk,
>> >> > + &cpu_clk,
>> >> > +};
>> >> > +
>> >> > +static struct regmap_config a1_cpu_regmap_cfg = {
>> >> > + .reg_bits = 32,
>> >> > + .val_bits = 32,
>> >> > + .reg_stride = 4,
>> >> > + .max_register = CPUCTRL_CLK_CTRL1,
>> >> > +};
>> >> > +
>> >> > +static struct meson_clk_hw_data a1_cpu_clks = {
>> >> > + .hws = a1_cpu_hw_clks,
>> >> > + .num = ARRAY_SIZE(a1_cpu_hw_clks),
>> >> > +};
>> >> > +
>> >> > +struct a1_cpu_clk_nb_data {
>> >> > + const struct clk_ops *mux_ops;
>> >>
>> >> That's fishy ...
>> >>
>> >> > + struct clk_hw *cpu_clk;
>> >> > + struct notifier_block nb;
>> >> > + u8 parent;
>> >> > +};
>> >> > +
>> >> > +#define MESON_A1_CPU_CLK_GET_PARENT(nbd) \
>> >> > + ((nbd)->mux_ops->get_parent((nbd)->cpu_clk))
>> >> > +#define MESON_A1_CPU_CLK_SET_PARENT(nbd, index) \
>> >> > + ((nbd)->mux_ops->set_parent((nbd)->cpu_clk, index))
>> >>
>> >> ... Directly going for the mux ops ??!?? No way !
>> >>
>> >> We have a framework to handle the clocks, the whole point is to use it,
>> >> not bypass it !
>> >>
>> >
>> > I suppose you understand my approach, which is quite similar to what is
>> > happening in the Mediatek driver:
>> >
>> > https://elixir.bootlin.com/linux/latest/source/drivers/clk/mediatek/clk-mux.c#L295
>> >
>> > Initially, I attempted to set the parent using the clk_set_parent() API.
>> > However, I encountered a problem with recursive calling of the
>> > notifier_block. This issue arises because the parent triggers
>> > notifications for its children, leading to repeated calls to the
>> > notifier_block.
>> >
>> > I find it puzzling why I cannot call an internal function or callback
>> > within the internal driver context. After all, the notifier block is
>> > just a part of the set_rate() flow. From a global Clock Control
>> > Framework perspective, the context should not change.
>>
>> I don't care what MTK is doing TBH. Forcefully calling ops on a clock,
>> hoping they are going to match with the clock type is wrong.
>>
>> There is a clk_hw API. Please it.
>>
>
> Yes, if sys_pll has a notifier_block instead of cpu_clk, there will be
> no problem with the clk_hw API.
>
> I will rework that point.
>
>> >
>> >> > +
>> >> > +static int meson_a1_cpu_clk_notifier_cb(struct notifier_block *nb,
>> >> > + unsigned long event, void *data)
>> >> > +{
>> >> > + struct a1_cpu_clk_nb_data *nbd;
>> >> > + int ret = 0;
>> >> > +
>> >> > + nbd = container_of(nb, struct a1_cpu_clk_nb_data, nb);
>> >> > +
>> >> > + switch (event) {
>> >> > + case PRE_RATE_CHANGE:
>> >> > + nbd->parent = MESON_A1_CPU_CLK_GET_PARENT(nbd);
>> >> > + /* Fallback to the CPU fixed clock */
>> >> > + ret = MESON_A1_CPU_CLK_SET_PARENT(nbd, 0);
>> >> > + /* Wait for clock propagation */
>> >> > + udelay(100);
>> >> > + break;
>> >> > +
>> >> > + case POST_RATE_CHANGE:
>> >> > + case ABORT_RATE_CHANGE:
>> >> > + /* Back to the original parent clock */
>> >> > + ret = MESON_A1_CPU_CLK_SET_PARENT(nbd, nbd->parent);
>> >> > + /* Wait for clock propagation */
>> >> > + udelay(100);
>> >> > + break;
>> >> > +
>> >> > + default:
>> >> > + pr_warn("Unknown event %lu for %s notifier block\n",
>> >> > + event, clk_hw_get_name(nbd->cpu_clk));
>> >> > + break;
>> >> > + }
>> >> > +
>> >> > + return notifier_from_errno(ret);
>> >> > +}
>> >> > +
>> >> > +static struct a1_cpu_clk_nb_data a1_cpu_clk_nb_data = {
>> >> > + .mux_ops = &clk_regmap_mux_ops,
>> >> > + .cpu_clk = &cpu_clk.hw,
>> >> > + .nb.notifier_call = meson_a1_cpu_clk_notifier_cb,
>> >> > +};
>> >> > +
>> >> > +static int meson_a1_dvfs_setup(struct platform_device *pdev)
>> >> > +{
>> >> > + struct device *dev = &pdev->dev;
>> >> > + struct clk *notifier_clk;
>> >> > + int ret;
>> >> > +
>> >> > + /* Setup clock notifier for cpu_clk */
>> >> > + notifier_clk = devm_clk_hw_get_clk(dev, &cpu_clk.hw, "dvfs");
>> >> > + if (IS_ERR(notifier_clk))
>> >> > + return dev_err_probe(dev, PTR_ERR(notifier_clk),
>> >> > + "can't get cpu_clk as notifier clock\n");
>> >> > +
>> >> > + ret = devm_clk_notifier_register(dev, notifier_clk,
>> >> > + &a1_cpu_clk_nb_data.nb);
>> >> > + if (ret)
>> >> > + return dev_err_probe(dev, ret,
>> >> > + "can't register cpu_clk notifier\n");
>> >> > +
>> >> > + return ret;
>> >> > +}
>> >> > +
>> >> > +static int meson_a1_cpu_probe(struct platform_device *pdev)
>> >> > +{
>> >> > + struct device *dev = &pdev->dev;
>> >> > + void __iomem *base;
>> >> > + struct regmap *map;
>> >> > + int clkid, i, err;
>> >> > +
>> >> > + base = devm_platform_ioremap_resource(pdev, 0);
>> >> > + if (IS_ERR(base))
>> >> > + return dev_err_probe(dev, PTR_ERR(base),
>> >> > + "can't ioremap resource\n");
>> >> > +
>> >> > + map = devm_regmap_init_mmio(dev, base, &a1_cpu_regmap_cfg);
>> >> > + if (IS_ERR(map))
>> >> > + return dev_err_probe(dev, PTR_ERR(map),
>> >> > + "can't init regmap mmio region\n");
>> >> > +
>> >> > + /* Populate regmap for the regmap backed clocks */
>> >> > + for (i = 0; i < ARRAY_SIZE(a1_cpu_regmaps); i++)
>> >> > + a1_cpu_regmaps[i]->map = map;
>> >> > +
>> >> > + for (clkid = 0; clkid < a1_cpu_clks.num; clkid++) {
>> >> > + err = devm_clk_hw_register(dev, a1_cpu_clks.hws[clkid]);
>> >> > + if (err)
>> >> > + return dev_err_probe(dev, err,
>> >> > + "clock[%d] registration failed\n",
>> >> > + clkid);
>> >> > + }
>> >> > +
>> >> > + err = devm_of_clk_add_hw_provider(dev, meson_clk_hw_get, &a1_cpu_clks);
>> >> > + if (err)
>> >> > + return dev_err_probe(dev, err, "can't add clk hw provider\n");
>> >>
>> >> I wonder if there is a window of opportunity to poke the syspll without
>> >> your notifier here. That being said, the situation would be similar on g12.
>> >>
>> >
>> > Yes, I have taken into account what you did in the G12A CPU clock
>> > relations. My thoughts were that it might not be applicable for the A1
>> > case. This is because the sys_pll should be located in a different
>> > driver from a logical perspective. Consequently, we cannot configure the
>> > sys_pll notifier block to manage the cpu_clk from a different driver.
>> > However, if I were to move the sys_pll clock object to the A1 CPU clock
>> > controller, I believe the g12a sys_pll notifier approach would work.
>> >
>>
>> I fail to see the connection between the number of device and the
>> approach you took.
>>
>>
>> >> > +
>> >> > + return meson_a1_dvfs_setup(pdev);
>> >>
>> >>
>> >>
>> >> > +}
>> >> > +
>> >> > +static const struct of_device_id a1_cpu_clkc_match_table[] = {
>> >> > + { .compatible = "amlogic,a1-cpu-clkc", },
>> >> > + {}
>> >> > +};
>> >> > +MODULE_DEVICE_TABLE(of, a1_cpu_clkc_match_table);
>> >> > +
>> >> > +static struct platform_driver a1_cpu_clkc_driver = {
>> >> > + .probe = meson_a1_cpu_probe,
>> >> > + .driver = {
>> >> > + .name = "a1-cpu-clkc",
>> >> > + .of_match_table = a1_cpu_clkc_match_table,
>> >> > + },
>> >> > +};
>> >> > +
>> >> > +module_platform_driver(a1_cpu_clkc_driver);
>> >> > +MODULE_AUTHOR("Dmitry Rokosov <ddrokosov@salutedevices.com>");
>> >> > +MODULE_LICENSE("GPL");
>> >> > diff --git a/drivers/clk/meson/a1-cpu.h b/drivers/clk/meson/a1-cpu.h
>> >> > new file mode 100644
>> >> > index 000000000000..e9af4117e26f
>> >> > --- /dev/null
>> >> > +++ b/drivers/clk/meson/a1-cpu.h
>> >>
>> >> There is not point putting the definition here in a header
>> >> These are clearly not going to be shared with another driver.
>> >>
>> >> Please drop this file
>> >>
>> >
>> > The same approach was applied to the Peripherals and PLL A1 drivers.
>> > Honestly, I am not a fan of having different file organization within a
>> > single logical code folder.
>> >
>> > Please refer to:
>> >
>> > drivers/clk/meson/a1-peripherals.h
>> > drivers/clk/meson/a1-pll.h
>>
>> I understand. There was a time where it made sense, some definition
>> could have been shared back then. This is no longer the case and it
>> would probably welcome a rework.
>>
>> ... and again, just pointing to another code does really invalidate my
>> point.
>>
>> >
>> >> > @@ -0,0 +1,16 @@
>> >> > +/* SPDX-License-Identifier: GPL-2.0+ */
>> >> > +/*
>> >> > + * Amlogic A1 CPU Clock Controller internals
>> >> > + *
>> >> > + * Copyright (c) 2024, SaluteDevices. All Rights Reserved.
>> >> > + * Author: Dmitry Rokosov <ddrokosov@salutedevices.com>
>> >> > + */
>> >> > +
>> >> > +#ifndef __A1_CPU_H
>> >> > +#define __A1_CPU_H
>> >> > +
>> >> > +/* cpu clock controller register offset */
>> >> > +#define CPUCTRL_CLK_CTRL0 0x80
>> >> > +#define CPUCTRL_CLK_CTRL1 0x84
>> >>
>> >> You are claiming the registers from 0x00 to 0x84 (included), but only
>> >> using these 2 registers ? What is the rest ? Are you sure there is only
>> >> clocks in there ?
>> >>
>> >
>> > Yes, unfortunately, the register map for this IP is not described in the
>> > A1 Datasheet. The only available information about it can be found in
>> > the vendor clock driver, which provides details for only two registers
>> > used to configure the CPU clock.
>> >
>> > From vendor kernel dtsi:
>> >
>> > clkc: clock-controller {
>> > compatible = "amlogic,a1-clkc";
>> > #clock-cells = <1>;
>> > reg = <0x0 0xfe000800 0x0 0x100>,
>> > <0x0 0xfe007c00 0x0 0x21c>,
>> > <0x0 0xfd000000 0x0 0x88>; <==== CPU clock regmap
>> > reg-names = "basic", "pll",
>> > "cpu_clk";
>> > clocks = <&xtal>;
>> > clock-names = "core";
>> > status = "okay";
>> > };
>> >
>> > From vendor clkc driver:
>> >
>> > /*
>> > * CPU clok register offset
>> > * APB_BASE: APB1_BASE_ADDR = 0xfd000000
>> > */
>> >
>> > #define CPUCTRL_CLK_CTRL0 0x80
>> > #define CPUCTRL_CLK_CTRL1 0x84
>>
>> If you had clock in 0x0 and 0x80, then I would agree claiming 0x0-0x88
>> is reasonable, even if you don't really know what is in between. It
>> would be a fair assumption.
>>
>> It is not the case here.
>> For all we know it could resets, power domains, etc ...
>>
>
> However, we do not have any information indicating that the gap from
> 0x00-0x80 contains additional registers. It is possible that there are
> internal registers, but they are not mentioned in the vendor datasheet
> or driver. Therefore, in this case, I personally prefer to rely on the
> vendor mapping.
I understand your preference. My request remains.
--
Jerome
^ permalink raw reply
* Re: [PATCH v3] clk: starfive: pll: Fix lower rate of CPUfreq by setting PLL0 rate to 1.5GHz
From: Krzysztof Kozlowski @ 2024-04-02 16:18 UTC (permalink / raw)
To: Xingyu Wu, Michael Turquette, Stephen Boyd, Conor Dooley,
Emil Renner Berthing, Rob Herring, Krzysztof Kozlowski
Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Hal Feng, linux-kernel,
linux-clk, linux-riscv, devicetree
In-Reply-To: <20240402090920.11627-1-xingyu.wu@starfivetech.com>
On 02/04/2024 11:09, Xingyu Wu wrote:
> CPUfreq supports 4 cpu frequency loads on 375/500/750/1500MHz.
> But now PLL0 rate is 1GHz and the cpu frequency loads become
> 333/500/500/1000MHz in fact.
>
> So PLL0 rate should be default set to 1.5GHz. But setting the
> PLL0 rate need certain steps:
>
> 1. Change the parent of cpu_root clock to OSC clock.
> 2. Change the divider of cpu_core if PLL0 rate is higher than
> 1.25GHz before CPUfreq boot.
> 3. Change the parent of cpu_root clock back to PLL0 clock.
>
> Reviewed-by: Hal Feng <hal.feng@starfivetech.com>
> Fixes: e2c510d6d630 ("riscv: dts: starfive: Add cpu scaling for JH7110 SoC")
> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>
> ---
>
> Hi Stephen and Emil,
>
> This patch fixes the issue about lower rate of CPUfreq[1] by setting PLL0
> rate to 1.5GHz.
>
> In order not to affect the cpu operation, setting the PLL0 rate need
> certain steps. The cpu_root's parent clock should be changed first. And
> the divider of the cpu_core clock should be set to 2 so they won't crash
> when setting 1.5GHz without voltage regulation. Due to PLL driver boot
> earlier than SYSCRG driver, cpu_core and cpu_root clocks are using by
> ioremap().
>
> [1]: https://github.com/starfive-tech/VisionFive2/issues/55
>
> Previous patch link:
> v2: https://lore.kernel.org/all/20230821152915.208366-1-xingyu.wu@starfivetech.com/
> v1: https://lore.kernel.org/all/20230811033631.160912-1-xingyu.wu@starfivetech.com/
>
> Thanks,
> Xingyu Wu
> ---
> .../jh7110-starfive-visionfive-2.dtsi | 5 +
> .../clk/starfive/clk-starfive-jh7110-pll.c | 102 ++++++++++++++++++
Please do not mix DTS and driver code. That's not really portable. DTS
is being exported and used in other projects.
...
>
> @@ -458,6 +535,8 @@ static int jh7110_pll_probe(struct platform_device *pdev)
> struct jh7110_pll_priv *priv;
> unsigned int idx;
> int ret;
> + struct device_node *np;
> + struct resource res;
>
> priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> if (!priv)
> @@ -489,6 +568,29 @@ static int jh7110_pll_probe(struct platform_device *pdev)
> return ret;
> }
>
> + priv->is_first_set = true;
> + np = of_find_compatible_node(NULL, NULL, "starfive,jh7110-syscrg");
Your drivers should not do it. It's fragile, hides true link/dependency.
Please use phandles.
> + if (!np) {
> + ret = PTR_ERR(np);
> + dev_err(priv->dev, "failed to get syscrg node\n");
> + goto np_put;
> + }
> +
> + ret = of_address_to_resource(np, 0, &res);
> + if (ret) {
> + dev_err(priv->dev, "failed to get syscrg resource\n");
> + goto np_put;
> + }
> +
> + priv->syscrg_base = ioremap(res.start, resource_size(&res));
> + if (!priv->syscrg_base)
> + ret = -ENOMEM;
Why are you mapping other device's IO? How are you going to ensure
synced access to registers?
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH] arm64: dts: ti: k3-j722s: Disable ethernet ports by default
From: Michael Walle @ 2024-04-02 16:16 UTC (permalink / raw)
To: Nishanth Menon
Cc: Vignesh Raghavendra, Tero Kristo, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-arm-kernel, devicetree,
linux-kernel
In-Reply-To: <20240402161306.tkg35heqlwqxoaua@another>
[-- Attachment #1: Type: text/plain, Size: 176 bytes --]
Hi,
> This is better at arch/arm64/boot/dts/ti/k3-am62p-main.dtsi.
I'm fine with that, but be aware that we'll also change the am62p
SoC dtsi in that case.
-michael
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 297 bytes --]
^ permalink raw reply
* Re: [PATCH v5 0/5] PCI: dwc: Add common pme_turn_off message by using outbound iATU
From: Frank Li @ 2024-04-02 16:15 UTC (permalink / raw)
To: Bjorn Helgaas, Jingoo Han, Gustavo Pimentel,
Manivannan Sadhasivam, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, imx
Cc: linux-pci, linux-kernel, devicetree, Yoshihiro Shimoda,
Serge Semin
In-Reply-To: <20240319-pme_msg-v5-0-af9ffe57f432@nxp.com>
On Tue, Mar 19, 2024 at 12:07:10PM -0400, Frank Li wrote:
> Involve an new and common mathod to send pme_turn_off() message. Previously
> pme_turn_off() implement by platform related special register to trigge
> it.
>
> But Yoshihiro give good idea by using iATU to send out message. Previously
> Yoshihiro provide patches to raise INTx message by dummy write to outbound
> iATU.
>
> Use similar mathod to send out pme_turn_off message.
>
> Previous two patches is picked from Yoshihiro' big patch serialise.
> PCI: dwc: Change arguments of dw_pcie_prog_outbound_atu()
> PCI: Add INTx Mechanism Messages macros
>
> PCI: Add PME_TURN_OFF message macro
> dt-bindings: PCI: dwc: Add 'msg" register region, Add "msg" region to use
> to map PCI msg.
>
> PCI: dwc: Add common pme_turn_off message method
> Using common pme_turn_off() message if platform have not define their.
>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> Changes in v5:
> - Default disable allocate TLP message memory windows. If driver need use
> this feature, need set use_atu_msg = true before call dw_host_init().
Mani, lorenzo and bjorn
Any comments about these patches? I already set this feature default as
false. It is common for all dwc platform.
After this merge, imx6 and layerscape many customer PME code can be
removed.
Frank
>
> - Link to v4: https://lore.kernel.org/r/20240213-pme_msg-v4-0-e2acd4d7a292@nxp.com
>
> Changes in v4:
> - Remove dt-binding patch. Needn't change any dts file and binding doc.
> Reserve a region at end of first IORESOURCE_MEM window by call
> request_resource(). So PCIe stack will not use this reserve region to any
> PCIe devices.
> I tested it by reserve at begin of IORESOURCE_MEM window. PCIe stack
> will skip it as expection.
>
> Fixed a issue, forget set iATU index when sent PME_turn_off.
>
> - Link to v3: https://lore.kernel.org/r/20240202-pme_msg-v3-0-ff2af57a02ad@nxp.com
>
> Changes in v3:
> - fix 'MSG"
> - Add pcie spec ref in head file
> - using function name dw_pci_pme_turn_off()
> - Using PCIE_ prefix macro
> - Link to v2: https://lore.kernel.org/r/20240201-pme_msg-v2-0-6767052fe6a4@nxp.com
>
> Changes in v2:
> - Add my sign off at PCI: dwc: Add outbound MSG TLPs support
> - Add Bjorn review tag at Add INTx Mechanism Messages macros
> - using PME_Turn_Off match PCIe spec
> - ref to pcie spec v6.1
> - using section number.
>
> - Link to v1: https://lore.kernel.org/r/20240130-pme_msg-v1-0-d52b0add5c7c@nxp.com
>
> ---
> Frank Li (2):
> PCI: Add PCIE_MSG_CODE_PME_TURN_OFF message macro
> PCI: dwc: Add common send PME_Turn_Off message method
>
> Yoshihiro Shimoda (3):
> PCI: Add INTx Mechanism Messages macros
> PCI: dwc: Consolidate args of dw_pcie_prog_outbound_atu() into a structure
> PCI: dwc: Add outbound MSG TLPs support
>
> drivers/pci/controller/dwc/pcie-designware-ep.c | 21 ++--
> drivers/pci/controller/dwc/pcie-designware-host.c | 146 +++++++++++++++++++---
> drivers/pci/controller/dwc/pcie-designware.c | 54 ++++----
> drivers/pci/controller/dwc/pcie-designware.h | 22 +++-
> drivers/pci/pci.h | 20 +++
> 5 files changed, 199 insertions(+), 64 deletions(-)
> ---
> base-commit: e08fc59eee9991afa467d406d684d46d543299a9
> change-id: 20240130-pme_msg-dd2d81ee9886
>
> Best regards,
> ---
> Frank Li <Frank.Li@nxp.com>
>
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: phy: qmp-ufs: Fix PHY clocks for SC7180
From: Rob Herring @ 2024-04-02 16:15 UTC (permalink / raw)
To: Danila Tikhonov
Cc: davidwronek, manivannan.sadhasivam, krzysztof.kozlowski+dt,
linux-arm-msm, konrad.dybcio, linux-phy, conor+dt, kishon,
devicetree, andersson, vkoul, linux-kernel,
cros-qcom-dts-watchers
In-Reply-To: <20240401182240.55282-2-danila@jiaxyga.com>
On Mon, 01 Apr 2024 21:22:39 +0300, Danila Tikhonov wrote:
> QMP UFS PHY used in SC7180 requires 3 clocks:
>
> * ref - 19.2MHz reference clock from RPMh
> * ref_aux - Auxiliary reference clock from GCC
> * qref - QREF clock from GCC
>
> This change obviously breaks the ABI, but it is inevitable since the
> clock topology needs to be accurately described in the binding.
>
> Signed-off-by: Danila Tikhonov <danila@jiaxyga.com>
> ---
> .../devicetree/bindings/phy/qcom,sc8280xp-qmp-ufs-phy.yaml | 1 +
> 1 file changed, 1 insertion(+)
>
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [RFC PATCH 5/6] watchdog: ROHM BD96801 PMIC WDG driver
From: Krzysztof Kozlowski @ 2024-04-02 16:15 UTC (permalink / raw)
To: Matti Vaittinen, Matti Vaittinen
Cc: Lee Jones, Wim Van Sebroeck, Guenter Roeck, devicetree,
linux-kernel, linux-watchdog
In-Reply-To: <f8e743a6c49607de0dd7a27778383477e051b130.1712058690.git.mazziesaccount@gmail.com>
On 02/04/2024 15:11, Matti Vaittinen wrote:
> Introduce driver for WDG block on ROHM BD96801 scalable PMIC.
>
> This driver only supports watchdog with I2C feeding and delayed
> response detection. Whether the watchdog toggles PRSTB pin or
> just causes an interrupt can be configured via device-tree.
>
> The BD96801 PMIC HW supports also window watchdog (too early
...
> +
> +static struct platform_driver bd96801_wdt = {
> + .driver = {
> + .name = "bd96801-wdt"
> + },
> + .probe = bd96801_wdt_probe,
> +};
> +module_platform_driver(bd96801_wdt);
> +
> +MODULE_AUTHOR("Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>");
> +MODULE_DESCRIPTION("BD96801 watchdog driver");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:bd96801-wdt");
Same comment on alias. Please use ID table.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 2/2] arm64: dts: ti: k3-am642-phyboard-electra-rdk: Increase CAN max bitrate
From: Nishanth Menon @ 2024-04-02 16:14 UTC (permalink / raw)
To: Nathan Morrisson
Cc: vigneshr, kristo, robh, krzysztof.kozlowski+dt, conor+dt,
linux-arm-kernel, devicetree, linux-kernel, upstream, w.egorov
In-Reply-To: <20240402161203.q34gyqfaoewvjbky@unburned>
On 11:12-20240402, Nishanth Menon wrote:
> On 09:08-20240402, Nathan Morrisson wrote:
> > The phyBOARD-Electra has two TCAN1044VDD CAN transceivers which
> > support CAN FD at 8 Mbps.
> >
> > Increase the maximum bitrate to 8 Mbps.
> >
> > Signed-off-by: Nathan Morrisson <nmorrisson@phytec.com>
> > ---
> > arch/arm64/boot/dts/ti/k3-am642-phyboard-electra-rdk.dts | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/ti/k3-am642-phyboard-electra-rdk.dts b/arch/arm64/boot/dts/ti/k3-am642-phyboard-electra-rdk.dts
> > index 8237b8c815b8..522699ec65e8 100644
> > --- a/arch/arm64/boot/dts/ti/k3-am642-phyboard-electra-rdk.dts
> > +++ b/arch/arm64/boot/dts/ti/k3-am642-phyboard-electra-rdk.dts
> > @@ -42,7 +42,7 @@ can_tc1: can-phy0 {
> > pinctrl-names = "default";
> > pinctrl-0 = <&can_tc1_pins_default>;
> > #phy-cells = <0>;
> > - max-bitrate = <5000000>;
> > + max-bitrate = <8000000>;
> > standby-gpios = <&main_gpio0 32 GPIO_ACTIVE_HIGH>;
> > };
> >
> > @@ -51,7 +51,7 @@ can_tc2: can-phy1 {
> > pinctrl-names = "default";
> > pinctrl-0 = <&can_tc2_pins_default>;
> > #phy-cells = <0>;
> > - max-bitrate = <5000000>;
> > + max-bitrate = <8000000>;
> > standby-gpios = <&main_gpio0 35 GPIO_ACTIVE_HIGH>;
> > };
> >
> > --
> > 2.25.1
> >
>
> This is better at arch/arm64/boot/dts/ti/k3-am62p-main.dtsi.
>
Woops.. wrong mail thread. :( Apologies on the spam.
--
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D
^ 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