Re: [PATCH v5 3/3] rtc: s5m: check the return value of s5m8767_rtc_init_reg()
On Thu, Jan 14, 2021 at 11:22:19AM +0100, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski > > This function can fail if regmap operations fail so check its return > value in probe(). > > Signed-off-by: Bartosz Golaszewski > --- > drivers/rtc/rtc-s5m.c | 2 ++ > 1 file changed, 2 insertions(+) Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH] soc: samsung: pm_domains: Convert to regular platform driver
On Thu, Jan 14, 2021 at 11:07:30AM -0800, Saravana Kannan wrote:
> On Thu, Jan 14, 2021 at 11:03 AM Marek Szyprowski
> wrote:
> >
> > Hi Saravana,
> >
> > On 13.01.2021 21:27, Saravana Kannan wrote:
> > > On Wed, Jan 13, 2021 at 3:03 AM Marek Szyprowski
> > > wrote:
> > >> When Exynos power domain driver was introduced, the only way to ensure
> > >> that power domains will be instantiated before the devices which belongs
> > >> to them was to initialize them early enough, before the devices are
> > >> instantiated in the system. This in turn required not to use any platform
> > >> device infrastructure at all, as there have been no way to ensure proper
> > >> probe order between devices.
> > >>
> > >> This has been finally changed and patch e590474768f1 ("driver core: Set
> > >> fw_devlink=on by default") ensures that each device will be probbed only
> > >> when its resource providers are ready. This allows to convert Exynos
> > >> power domain driver to regular platform driver.
> > >>
> > >> This is also required by the mentioned commit to enable probing any
> > >> device which belongs to the Exynos power domains, as otherwise the core
> > >> won't notice that the power domains are in fact available.
> > >>
> > >> Signed-off-by: Marek Szyprowski
> > >> ---
> > >> Some more comments are in the following thread:
> > >> https://protect2.fireeye.com/v1/url?k=8ac052ac-d55b6ba4-8ac1d9e3-0cc47a31c8b4-9068b559b0fd155d=1=b393c3ff-16ba-48a4-9d72-6805d02971d5=https%3A%2F%2Flore.kernel.org%2Flkml%2F2556a69b-5da5-bf80-e051-df2d02fbc40f%40samsung.com%2F
> > >> ---
> > >> ...
> > > Skimmed through this patch and at a high level, it looks good for what
> > > it's trying to do. Thanks for doing this!
> > >
> > > Btw, I assume that this won't work with fw_devlink=off/permissive
> > > (default since 5.10 or earlier)? My concern is that we might
> > > temporarily set fw_devlink=permissive by default if the other
> > > breakages aren't fixed in time for 5.12? How do you want to handle that?
> >
> > I've applied my patch on top of vanilla v5.10 and checked on my test
> > boards. Surprisingly everything works fine, so something must have been
> > changed during the last few years as the power domain driver in the
> > current form has been written long time ago. I remember that the moment
> > when platform devices are created from the of nodes has been change at
> > some point, so maybe this is somehow related. Anyway, the platform
> > driver for Exynos power domains registered from core_initcall works fine
> > with v5.10 kernel.
> >
> > I have no strong opinion on the way of merging this fix. It can go via
> > Samsung tree, so in the end the v5.12-rc1 will have both my fix and your
> > change, but won't be fully bisectable in-between. Krzysztof, what's your
> > opinion?
>
> If it doesn't break anything without my changes, then let's try to get
> it merged independent of my series. This is a good change even without
> my changes.
I agree, I'll take it via Samsung SoC. It's not the perfect solution -
as Marek said, the tree won't be bisectable. Have in mind that some
other boards/architectures might be broken where no one reported it yet,
so fw_devlink=off/permissive might still be good choice for v5.12.
Best regards,
Krzysztof
Re: [PATCH v2 2/3] arm64: dts: imx: Add i.mx8mm nitrogen8mm basic dts support
On Wed, Jan 13, 2021 at 03:34:42PM +0100, Adrien Grassein wrote:
> Tested with a basic Build Root configuration booting from sdcard.
>
> Signed-off-by: Adrien Grassein
> ---
> arch/arm64/boot/dts/freescale/Makefile| 1 +
> .../dts/freescale/imx8mm-nitrogen8mm_rev2.dts | 364 ++
> 2 files changed, 365 insertions(+)
> create mode 100644 arch/arm64/boot/dts/freescale/imx8mm-nitrogen8mm_rev2.dts
>
> diff --git a/arch/arm64/boot/dts/freescale/Makefile
> b/arch/arm64/boot/dts/freescale/Makefile
> index 901d80086b47..b2eb7a5e4db3 100644
> --- a/arch/arm64/boot/dts/freescale/Makefile
> +++ b/arch/arm64/boot/dts/freescale/Makefile
> @@ -45,6 +45,7 @@ dtb-$(CONFIG_ARCH_MXC) += imx8mq-librem5-devkit.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mq-librem5-r2.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mq-librem5-r3.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mq-nitrogen.dtb
> +dtb-$(CONFIG_ARCH_MXC) += imx8mm-nitrogen8mm_rev2.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mq-phanbell.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mq-pico-pi.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mq-thor96.dtb
> diff --git a/arch/arm64/boot/dts/freescale/imx8mm-nitrogen8mm_rev2.dts
> b/arch/arm64/boot/dts/freescale/imx8mm-nitrogen8mm_rev2.dts
> new file mode 100644
> index ..506e467ebf16
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx8mm-nitrogen8mm_rev2.dts
> @@ -0,0 +1,364 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Device Tree file for Boundary Devices i.MX8MMini Nitrogen8MM Rev2 board.
> + * Adrien Grassein
> + */
> +/dts-v1/;
> +#include "imx8mm.dtsi"
> +
> +/ {
> + model = "Boundary Devices i.MX8MMini Nitrogen8MM Rev2";
> + compatible = "boundary,imx8mm-nitrogen8mm", "fsl,imx8mm";
> +
> + reg_vref_1v8: regulator-vref-1v8 {
> + compatible = "regulator-fixed";
> + regulator-name = "vref-1v8";
> + regulator-min-microvolt = <180>;
> + regulator-max-microvolt = <180>;
> + };
> +
> + reg_vref_3v3: regulator-vref-3v3 {
> + compatible = "regulator-fixed";
> + regulator-name = "vref-3v3";
> + regulator-min-microvolt = <330>;
> + regulator-max-microvolt = <330>;
> + };
Why do you need these two regulators? They don't do anything, there is
no control over them.
> +};
> +
> +_0 {
> + cpu-supply = <_sw3>;
> +};
> +
> +_1 {
> + cpu-supply = <_sw3>;
> +};
> +
> +_2 {
> + cpu-supply = <_sw3>;
> +};
> +
> +_3 {
> + cpu-supply = <_sw3>;
> +};
> +
> + {
> + clock-frequency = <40>;
> + pinctrl-names = "default", "gpio";
> + pinctrl-0 = <_i2c1>;
> + pinctrl-1 = <_i2c1_1>;
> + scl-gpios = < 14 GPIO_OPEN_DRAIN>;
> + sda-gpios = < 15 GPIO_OPEN_DRAIN>;
> + status = "okay";
> +
> + pmic@8 {
> + compatible = "nxp,pf8121a";
> + reg = <0x8>;
> + status = "okay";
> +
> + regulators {
> + reg_ldo2: ldo2 {
The PMIC regulators should be described fully, so bring them back from
the v1.
> + regulator-always-on;
> + regulator-boot-on;
> + regulator-max-microvolt = <500>;
> + regulator-min-microvolt = <150>;
> + };
> +
> + reg_sw3: buck3 {
> + regulator-always-on;
> + regulator-boot-on;
> + regulator-max-microvolt = <180>;
> + regulator-min-microvolt = <40>;
> + };
> + };
> + };
> +};
> +
> + {
Please order the overridden nodes alphabetically, with iomux exception
which goes to the end (by convention in NXP). You define i2c1, then fec1
and then i2c3. Should be fec1, i2c1 and then i2c3.
Best regards,
Krzysztof
Re: [PATCH v2 1/3] dt-bindings: arm: imx: add imx8mm nitrogen support
On Wed, Jan 13, 2021 at 03:34:41PM +0100, Adrien Grassein wrote: > The Nitrogen8M Mini is an ARM based single board computer (SBC). > > Signed-off-by: Adrien Grassein > --- > Documentation/devicetree/bindings/arm/fsl.yaml | 1 + > 1 file changed, 1 insertion(+) > Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH] arm64: dts: imx8mn-beacon-som: Configure RTC aliases
On Sun, Jan 10, 2021 at 05:53:53AM -0600, Adam Ford wrote: > On the i.MX8MN Beacon SOM, there is an RTC chip which is fed power > from the baseboard during power off. The SNVS RTC integrated into > the SoC is not fed power. Depending on the order the modules are > loaded, this can be a problem if the external RTC isn't rtc0. > > Make the alias for rtc0 point to the external RTC all the time and > rtc1 point to the SVNS in order to correctly hold date/time over > a power-cycle. > > Signed-off-by: Adam Ford Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH] arm64: dts: imx8mm-beacon: add more pinctrl states for usdhc1
On Sun, Jan 10, 2021 at 05:38:26AM -0600, Adam Ford wrote: > The WiFi chip is capable of communication at SDR104 speeds. > Enable 100Mhz and 200MHz pinmux to support this. > > Signed-off-by: Adam Ford > Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH] ARM: dts: exynos: Add energy model for Exynos 5250
On Mon, Jan 11, 2021 at 03:57:16PM -0800, Danny Lin wrote:
> This energy model enables the use of energy- and capacity-aware CPU
> frequency scaling.
>
> Power and performance measurements were made using my freqbench [1]
> benchmark coordinator, which uses EEMBC CoreMark [2] as the workload
> and measures power usage using the integrated PMIC's fuel gauge (DS2784
> in this case).
Thanks for the patch.
The tested setup is not representative here. The Nexus 10 is not
supported by mainline and it might use specific revision of Exynos5250,
different than present on other mainlined boards. One could assume that
ratio of dynamic power coefficient of the cores should be similar... but
there is no ratio here, as this is not a big.LITTLE platform.
Another problem is the v3.4 vendor kernel with a lot of out-of-tree
code. This means it might use some different scheduler, different
drivers, different voltages and a lot more of unknown differences.
Vendor kernel should not matter that much in measurement of DPC but it
makes the results not possible to reproduce.
You were also measuring the power delivered to entire system, no to CPUs,
so you included static power in the data. Static power of CPUs and of
the entire system.
> The energy model dynamic-power-coefficient values were calculated with
> DPC = µW / MHz / V^2
> for each OPP, and averaged across all OPPs within each cluster for the
> final coefficient.
>
> A Google Nexus 10 device running a downstream 3.4 kernel was used for
> benchmarking to ensure proper frequency scaling and other low-level
> controls.
>
> Raw benchmark results can be found in the freqbench repository [3].
> Below is a human-readable summary:
>
> = CPU 1 =
> Frequencies: 200 300 400 500 600 700 800 900 1000 1100 1200 1300 1400 1500
> 1600 1700
> 200: 909 4.5 C/MHz132 mW 36.2 J6.9 I/mJ 275.0 s
What are the columns here? I would expect that fuel gauge gives you the
current, but it's not here.
> 300: 1366 4.6 C/MHz212 mW 38.7 J6.5 I/mJ 183.0 s
> 400: 1821 4.6 C/MHz286 mW 39.3 J6.4 I/mJ 137.3 s
> 500: 2253 4.5 C/MHz375 mW 41.7 J6.0 I/mJ 111.0 s
> 600: 2740 4.6 C/MHz446 mW 40.7 J6.1 I/mJ91.2 s
> 700: 3199 4.6 C/MHz513 mW 40.1 J6.2 I/mJ78.2 s
> 800: 3673 4.6 C/MHz678 mW 46.1 J5.4 I/mJ68.1 s
> 900: 4090 4.5 C/MHz764 mW 46.7 J5.4 I/mJ61.1 s
> 1000: 4586 4.6 C/MHz878 mW 47.9 J5.2 I/mJ54.5 s
> 1100: 5060 4.6 C/MHz 1084 mW 53.6 J4.7 I/mJ49.4 s
> 1200: 5515 4.6 C/MHz 1225 mW 55.5 J4.5 I/mJ45.3 s
> 1300: 5933 4.6 C/MHz 1396 mW 58.9 J4.2 I/mJ42.1 s
> 1400: 6395 4.6 C/MHz 1662 mW 65.0 J3.8 I/mJ39.1 s
> 1500: 6897 4.6 C/MHz 1895 mW 68.7 J3.6 I/mJ36.3 s
> 1600: 7332 4.6 C/MHz 2198 mW 75.0 J3.3 I/mJ34.1 s
> 1700: 7826 4.6 C/MHz 2497 mW 79.8 J3.1 I/mJ31.9 s
>
> [1] https://github.com/kdrag0n/freqbench
> [2] https://www.eembc.org/coremark/
> [3] https://github.com/kdrag0n/freqbench/tree/master/results/exynos5250/main
>
> Signed-off-by: Danny Lin
> ---
> arch/arm/boot/dts/exynos5250.dtsi | 16
> 1 file changed, 16 insertions(+)
>
> diff --git a/arch/arm/boot/dts/exynos5250.dtsi
> b/arch/arm/boot/dts/exynos5250.dtsi
> index 2ea2caaca4e2..cc2fe0afcfc7 100644
> --- a/arch/arm/boot/dts/exynos5250.dtsi
> +++ b/arch/arm/boot/dts/exynos5250.dtsi
> @@ -58,6 +58,8 @@ cpu0: cpu@0 {
> clock-names = "cpu";
> operating-points-v2 = <_opp_table>;
> #cooling-cells = <2>; /* min followed by max */
> + capacity-dmips-mhz = <1024>;
The origin of this 1024 value should be explained.
> + dynamic-power-coefficient = <800>;
> };
> cpu1: cpu@1 {
> device_type = "cpu";
> @@ -67,6 +69,20 @@ cpu1: cpu@1 {
> clock-names = "cpu";
> operating-points-v2 = <_opp_table>;
> #cooling-cells = <2>; /* min followed by max */
> + capacity-dmips-mhz = <1024>;
> + dynamic-power-coefficient = <800>;
> + };
> +
> + cpu-map {
That's a second patch.
Best regards,
Krzysztof
> + cluster0 {
> + core0 {
> + cpu = <>;
> + };
> +
> + core1 {
> + cpu = <>;
> + };
> + };
> };
> };
>
> --
> 2.29.2
>
Re: [PATCH 8/9] arm64: dts: Add Librem5 Evergreen
On Tue, Jan 12, 2021 at 10:51:50AM +0100, Martin Kepplinger wrote: > Add librem5-r4 with specifics to that revision like the near-level, > battery and charger properties. For schematics and more information, > see https://developer.puri.sm/Librem5/Hardware_Reference/Evergreen.html > > Signed-off-by: Martin Kepplinger > --- > arch/arm64/boot/dts/freescale/Makefile| 1 + > .../boot/dts/freescale/imx8mq-librem5-r4.dts | 35 +++ > 2 files changed, 36 insertions(+) > create mode 100644 arch/arm64/boot/dts/freescale/imx8mq-librem5-r4.dts > The bindings go before DTS change using them. Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH 9/9] dt-bindings: arm: fsl: Add the librem 5 Evergreen revision
On Tue, Jan 12, 2021 at 10:51:51AM +0100, Martin Kepplinger wrote: > Add an entry for the Librem 5 phone, Evergreen revision which is supported > by "r4". Schematics and more information can be found at > https://developer.puri.sm/Librem5/Hardware_Reference/Evergreen.html > > Signed-off-by: Martin Kepplinger > --- > Documentation/devicetree/bindings/arm/fsl.yaml | 1 + > 1 file changed, 1 insertion(+) > Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH 2/9] arm64: dts: imx8mq-librem5: Mark charger IRQ as High-Z
On Tue, Jan 12, 2021 at 10:51:44AM +0100, Martin Kepplinger wrote: > From: Guido Günther > > This is consistent with other IRQs and makes keeps currents low. > > Signed-off-by: Guido Günther > Signed-off-by: Martin Kepplinger > --- > arch/arm64/boot/dts/freescale/imx8mq-librem5.dtsi | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH 1/9] arm64: defconfig: Enable vibra-pwm
On Tue, Jan 12, 2021 at 10:51:43AM +0100, Martin Kepplinger wrote: > From: Guido Günther > > The haptic motor for the Librem 5 uses this. > > Signed-off-by: Guido Günther > Signed-off-by: Martin Kepplinger > --- > arch/arm64/configs/defconfig | 1 + > 1 file changed, 1 insertion(+) > Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: pmwg/integ bisection: baseline.login on rk3328-rock64
On Tue, Jan 12, 2021 at 11:25:25AM +, Guillaume Tucker wrote: > Hi Vincent, > > Please see the bisection report below about a boot failure on > rk3328-rock64 with the pwmg/integ branch. > > Reports aren't automatically sent to the public while we're > trialing new bisection features on kernelci.org but this one > looks valid. > > There's nothing in the serial console log, probably because it's > crashing too early during boot. > > Some details can be found here: > > https://kernelci.org/test/case/id/5ffb978de38e717501c94cd8/ > > The bisection was run with CONFIG_RANDOMIZE_BASE=y enabled, but > the same issue occurs with a plain defconfig from that branch. > Results with other configs and platforms can be compared here: > > > https://kernelci.org/test/job/pmwg/branch/integ/kernel/v5.11-rc3-13-gcea05edf93998/plan/baseline/ > > Please let us know if you need some help to test a fix or debug > the issue. > > Thanks, > Guillaume > > > On 11/01/2021 05:36, KernelCI bot wrote: > > * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * > > * This automated bisection report was sent to you on the basis * > > * that you may be involved with the breaking commit it has * > > * found. No manual investigation has been done to verify it, * > > * and the root cause of the problem may be somewhere else. * > > * * > > * If you do send a fix, please include this trailer:* > > * Reported-by: "kernelci.org bot" * > > * * > > * Hope this helps! * > > * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * > > > > pmwg/integ bisection: baseline.login on rk3328-rock64 > > > > Summary: > > Start: cea05edf9399 Merge remote-tracking branch > > 'georgi.db845c/db845c-fixes' into integ > > Plain log: > > https://storage.kernelci.org/pmwg/integ/v5.11-rc3-13-gcea05edf93998/arm64/defconfig+CONFIG_RANDOMIZE_BASE=y/gcc-8/lab-baylibre/baseline-rk3328-rock64.txt > > HTML log: > > https://storage.kernelci.org/pmwg/integ/v5.11-rc3-13-gcea05edf93998/arm64/defconfig+CONFIG_RANDOMIZE_BASE=y/gcc-8/lab-baylibre/baseline-rk3328-rock64.html > > Result: 31379ec3d17b arm64/hikey: update defconfig > > > > Checks: > > revert: PASS > > verify: PASS > > > > Parameters: > > Tree: pmwg > > URL:https://git.linaro.org/power/linux.git > > Branch: integ > > Target: rk3328-rock64 > > CPU arch: arm64 > > Lab:lab-baylibre > > Compiler: gcc-8 > > Config: defconfig+CONFIG_RANDOMIZE_BASE=y > > Test case: baseline.login > > > > Breaking commit found: > > > > --- > > commit 31379ec3d17bf215585f1bac15eff77351830d37 > > Author: Vincent Guittot > > Date: Tue Nov 17 10:02:58 2020 +0100 > > > > arm64/hikey: update defconfig > > > > Signed-off-by: Vincent Guittot Hi all, Regardless of this bisected issue, such commit and its description is not good. Multiple entries are added and removed - no clue why. Was it savedefconfig? Maybe yes, maybe no, who knows... If yes, it should be reverted because savedefconfig is known to remove important features *later* (vide DEBUG_FS in the past). Instead, commit should explicitly explain for each item why this is added or removed. Best regards, Krzysztof > > > > diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig > > index 5cfe3cf6f2ac..4d2e85c7f96b 100644 > > --- a/arch/arm64/configs/defconfig > > +++ b/arch/arm64/configs/defconfig > > @@ -12,9 +12,11 @@ CONFIG_TASK_IO_ACCOUNTING=y > > CONFIG_IKCONFIG=y > > CONFIG_IKCONFIG_PROC=y > > CONFIG_NUMA_BALANCING=y > > +CONFIG_CGROUPS=y > > CONFIG_MEMCG=y > > CONFIG_MEMCG_SWAP=y > > CONFIG_BLK_CGROUP=y > > +CONFIG_CGROUP_SCHED=y > > CONFIG_CGROUP_PIDS=y > > CONFIG_CGROUP_HUGETLB=y > > CONFIG_CPUSETS=y > > @@ -22,7 +24,6 @@ CONFIG_CGROUP_DEVICE=y > > CONFIG_CGROUP_CPUACCT=y > > CONFIG_CGROUP_PERF=y > > CONFIG_USER_NS=y > > -CONFIG_SCHED_AUTOGROUP=y > > CONFIG_BLK_DEV_INITRD=y > > CONFIG_KALLSYMS_ALL=y > > # CONFIG_COMPAT_BRK is not set > > @@ -83,7 +84,6 @@ CONFIG_CPU_FREQ_GOV_POWERSAVE=m > > CONFIG_CPU_FREQ_GOV_USERSPACE=y > > CONFIG_CPU_FREQ_GOV_ONDEMAND=y > > CONFIG_CPU_FREQ_GOV_CONSERVATIVE=m > > -CONFIG_CPU_FREQ_GOV_SCHEDUTIL=y > > CONFIG_CPUFREQ_DT=y > > CONFIG_ACPI_CPPC_CPUFREQ=m > > CONFIG_ARM_ALLWINNER_SUN50I_CPUFREQ_NVMEM=m > > @@ -264,6 +264,7 @@ CONFIG_VIRTIO_BLK=y > > CONFIG_BLK_DEV_NVME=m > > CONFIG_SRAM=y > > CONFIG_PCI_ENDPOINT_TEST=m > > +CONFIG_HISI_HIKEY_USB=m > > CONFIG_EEPROM_AT24=m > > CONFIG_EEPROM_AT25=m > > CONFIG_UACCE=m > > @@ -768,9 +769,13 @@ CONFIG_USB_CONFIGFS_RNDIS=y > > CONFIG_USB_CONFIGFS_EEM=y > > CONFIG_USB_CONFIGFS_MASS_STORAGE=y > > CONFIG_USB_CONFIGFS_F_FS=y > >
Re: [PATCH v4 2/3] rtc: s5m: check the return value of s5m8767_rtc_init_reg()
On Mon, Jan 11, 2021 at 03:11:24PM +0100, Bartosz Golaszewski wrote: > On Mon, Jan 11, 2021 at 2:35 PM Krzysztof Kozlowski wrote: > > > > On Mon, Jan 11, 2021 at 01:40:26PM +0100, Bartosz Golaszewski wrote: > > > From: Bartosz Golaszewski > > > > > > This function can fail if regmap operations fail so check its return > > > value in probe(). > > > > > > Signed-off-by: Bartosz Golaszewski > > > --- > > > drivers/rtc/rtc-s5m.c | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > diff --git a/drivers/rtc/rtc-s5m.c b/drivers/rtc/rtc-s5m.c > > > index eb9dde4095a9..e0011d3cf61b 100644 > > > --- a/drivers/rtc/rtc-s5m.c > > > +++ b/drivers/rtc/rtc-s5m.c > > > @@ -791,6 +791,8 @@ static int s5m_rtc_probe(struct platform_device *pdev) > > > platform_set_drvdata(pdev, info); > > > > > > ret = s5m8767_rtc_init_reg(info); > > > + if (ret) > > > + return ret; > > > > You leak I2C device. > > > > Yes, the next patch fixes it but I changed the order. Actually this > can be moved after 3/3 with no conflicts when applying. Yes, but for bisecting and any backporting (e.g. with autosel) the order is quite important. Please resend with new order. Best regards, Krzysztof
Re: [PATCH v4 3/3] rtc: s5m: use devm_i2c_new_dummy_device()
On Mon, Jan 11, 2021 at 01:40:27PM +0100, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski > > Use the managed variant of i2c_new_dummy_device() to shrink code and > remove the goto label. We can drop the remove callback now too. > > Signed-off-by: Bartosz Golaszewski > --- > drivers/rtc/rtc-s5m.c | 31 +++ > 1 file changed, 7 insertions(+), 24 deletions(-) Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH v4 2/3] rtc: s5m: check the return value of s5m8767_rtc_init_reg()
On Mon, Jan 11, 2021 at 01:40:26PM +0100, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski > > This function can fail if regmap operations fail so check its return > value in probe(). > > Signed-off-by: Bartosz Golaszewski > --- > drivers/rtc/rtc-s5m.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/rtc/rtc-s5m.c b/drivers/rtc/rtc-s5m.c > index eb9dde4095a9..e0011d3cf61b 100644 > --- a/drivers/rtc/rtc-s5m.c > +++ b/drivers/rtc/rtc-s5m.c > @@ -791,6 +791,8 @@ static int s5m_rtc_probe(struct platform_device *pdev) > platform_set_drvdata(pdev, info); > > ret = s5m8767_rtc_init_reg(info); > + if (ret) > + return ret; You leak I2C device. Best regards, Krzysztof
Re: [PATCH v4 1/3] rtc: s5m: select REGMAP_I2C
On Mon, Jan 11, 2021 at 01:40:25PM +0100, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski
>
> The rtc-s5m uses the I2C regmap but doesn't select it in Kconfig so
> depending on the configuration the build may fail. Fix it.
>
> Signed-off-by: Bartosz Golaszewski
> ---
> drivers/rtc/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
>
Fixes: 959df7778bbd ("rtc: Enable compile testing for Maxim and Samsung
drivers")
Reviewed-by: Krzysztof Kozlowski
Best regards,
Krzysztof
Re: (subset) [PATCH] soc: samsung: exynos-chipid: correct helpers __init annotation
On Tue, 5 Jan 2021 18:44:40 +0100, Krzysztof Kozlowski wrote: > After converting to builtin driver, the probe function should not call > __init functions anymore: > > >> WARNING: modpost: vmlinux.o(.text+0x8884d4): > Section mismatch in reference from the function exynos_chipid_probe() to > the function .init.text:product_id_to_soc_id() Applied, thanks! [1/1] soc: samsung: exynos-chipid: correct helpers __init annotation commit: 6166174afc2bc74ca550af388508384b57d5163d Best regards, -- Krzysztof Kozlowski
[PATCH] soc: samsung: exynos-chipid: correct helpers __init annotation
After converting to builtin driver, the probe function should not call
__init functions anymore:
>> WARNING: modpost: vmlinux.o(.text+0x8884d4):
Section mismatch in reference from the function exynos_chipid_probe() to
the function .init.text:product_id_to_soc_id()
Reported-by: kernel test robot
Fixes: 352bfbb3e023 ("soc: samsung: exynos-chipid: convert to driver and merge
exynos-asv")
Signed-off-by: Krzysztof Kozlowski
---
drivers/soc/samsung/exynos-chipid.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/soc/samsung/exynos-chipid.c
b/drivers/soc/samsung/exynos-chipid.c
index fa6a9b9f6d70..5c1d0f97f766 100644
--- a/drivers/soc/samsung/exynos-chipid.c
+++ b/drivers/soc/samsung/exynos-chipid.c
@@ -44,7 +44,7 @@ static const struct exynos_soc_id {
{ "EXYNOS7420", 0xE742 },
};
-static const char * __init product_id_to_soc_id(unsigned int product_id)
+static const char *product_id_to_soc_id(unsigned int product_id)
{
int i;
--
2.25.1
Re: [PATCH v2 00/48] Introduce core voltage scaling for NVIDIA Tegra20/30 SoCs
On Thu, Dec 17, 2020 at 09:05:50PM +0300, Dmitry Osipenko wrote: > Introduce core voltage scaling for NVIDIA Tegra20/30 SoCs, which reduces > power consumption and heating of the Tegra chips. Tegra SoC has multiple > hardware units which belong to a core power domain of the SoC and share > the core voltage. The voltage must be selected in accordance to a minimum > requirement of every core hardware unit. > > The minimum core voltage requirement depends on: > > 1. Clock enable state of a hardware unit. > 2. Clock frequency. > 3. Unit's internal idling/active state. > > This series is tested on Acer A500 (T20), AC100 (T20), Nexus 7 (T30), > Ouya (T30), TK1 (T124) and some others. I also added voltage scaling to > the Ventana (T20) and Cardhu (T30) boards which are tested by NVIDIA's CI > farm. Tegra30 is now couple degrees cooler on Nexus 7 and stays cool on > Ouya (instead of becoming burning hot) while system is idling. It should > be possible to improve this further by implementing a more advanced power > management features for the kernel drivers. > > The DVFS support is opt-in for all boards, meaning that older DTBs will > continue to work like they did it before this series. It should be possible > to easily add the core voltage scaling support for Tegra114+ SoCs based on > this grounding work later on, if anyone will want to implement it. The same comment as for your interconnect work: for sets touching multiple systems please mention the dependencies between patches in the cover letter. Not as a reply to such remark like I make here, but as a separate entry in the cover letter. Best regards, Krzysztof
Re: [PATCH V2 2/4] memory: renesas rpc-if: Update Add RZ/G2 to Kconfig description
On Sat, Jan 02, 2021 at 05:54:10AM -0600, Adam Ford wrote: > The Renesas RPC-IF is present on the RZ/G2 Series. Add that to > the description. > > Suggested-by: Biju Das > Signed-off-by: Adam Ford > --- > drivers/memory/Kconfig | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > Thanks, applied with commit title change (renesas-rpc-if:) and fix pointed out by Geert. Best regards, Krzysztof
Re: [PATCH V2 1/4] dt-bindings: memory: Renesas RPC-IF: Add support for RZ/G2 Series
On Sat, Jan 02, 2021 at 05:54:09AM -0600, Adam Ford wrote: > The RZ/G2 Series has the RPC-IF interface. > Update bindings to support: r8a774a1, r8a774b1, r8a774c0, and r8a774e1 > > Signed-off-by: Adam Ford > --- > .../bindings/memory-controllers/renesas,rpc-if.yaml | 6 +- > 1 file changed, 5 insertions(+), 1 deletion(-) > > V2: Updated renesas,rcar-gen3-rpc-if to include RZ/G2 Thanks, applied. Best regards, Krzysztof
Re: [PATCH v12 3/5] memory: tegra124: Support interconnect framework
On Mon, Dec 28, 2020 at 06:49:18PM +0300, Dmitry Osipenko wrote: > Now Internal and External memory controllers are memory interconnection > providers. This allows us to use interconnect API for tuning of memory > configuration. EMC driver now supports OPPs and DVFS. > > Tested-by: Nicolas Chauvet > Acked-by: Georgi Djakov > Signed-off-by: Dmitry Osipenko > --- > drivers/memory/tegra/Kconfig| 1 + > drivers/memory/tegra/tegra124-emc.c | 320 +++- > drivers/memory/tegra/tegra124.c | 82 ++- > 3 files changed, 391 insertions(+), 12 deletions(-) Thanks, applied. Best regards, Krzysztof
Re: [PATCH v12 2/5] memory: tegra124-emc: Continue probing if timings are missing in device-tree
On Mon, Dec 28, 2020 at 06:49:17PM +0300, Dmitry Osipenko wrote: > EMC driver will become mandatory after turning it into interconnect > provider because interconnect users, like display controller driver, will > fail to probe using newer device-trees that have interconnect properties. > Thus make EMC driver to probe even if timings are missing in device-tree. > > Tested-by: Nicolas Chauvet > Signed-off-by: Dmitry Osipenko > --- > drivers/memory/tegra/tegra124-emc.c | 26 +- > 1 file changed, 9 insertions(+), 17 deletions(-) Thanks, applied. Best regards, Krzysztof
Re: [PATCH v12 1/5] memory: tegra124-emc: Make driver modular
On Mon, Dec 28, 2020 at 06:49:16PM +0300, Dmitry Osipenko wrote: > Add modularization support to the Tegra124 EMC driver, which now can be > compiled as a loadable kernel module. > > Note that EMC clock must be registered at clk-init time, otherwise PLLM > will be disabled as unused clock at boot time if EMC driver is compiled > as a module. Hence add a prepare/complete callbacks. similarly to what is > done for the Tegra20/30 EMC drivers. > > Tested-by: Nicolas Chauvet > Signed-off-by: Dmitry Osipenko > --- > drivers/clk/tegra/Kconfig| 3 ++ > drivers/clk/tegra/Makefile | 2 +- > drivers/clk/tegra/clk-tegra124-emc.c | 41 > drivers/clk/tegra/clk-tegra124.c | 26 -- > drivers/clk/tegra/clk.h | 18 > drivers/memory/tegra/Kconfig | 3 +- > drivers/memory/tegra/tegra124-emc.c | 31 ++--- > include/linux/clk/tegra.h| 8 ++ > include/soc/tegra/emc.h | 16 --- > 9 files changed, 106 insertions(+), 42 deletions(-) > delete mode 100644 include/soc/tegra/emc.h Thanks, applied. Best regards, Krzysztof
Re: [PATCH v6 2/8] regulator: dt-bindings: Document max8997-pmic nodes
On Mon, Jan 04, 2021 at 09:24:49PM +, Mark Brown wrote: > On Mon, Jan 04, 2021 at 07:38:21PM +0100, Krzysztof Kozlowski wrote: > > On Mon, Jan 04, 2021 at 06:27:34PM +, Mark Brown wrote: > > > > We can indicate the presence of features without adding new compatible > > > strings, that's just encoding the way Linux currently divides things up > > > into the bindings. For example having an extcon property seems like it > > > should be enough to figure out if we're using extcon. > > > It won't be enough because MFD will create device for extcon and bind > > the driver. The same for the charger. We have a board where max8997 is > > used only as PMIC (providing regulators) without battery and USB > > connectivity. > > I'm not sure I follow, sorry? Either the core driver can parse the > bindings enough to know what children it has or (probably better) it can > instantiate the children unconditionally and then the function drivers > can figure out if they need to do anything. Currently the MFD parent/core driver will instantiate children unconditionally. It would have to be adapted. With proposed bindings - nothing to change. MFD core already does the thing. The point is that function drivers should not be even bound, should not start to probe. Otherwise if they probe and fail, they will pollute the dmesg/probe log with failure. With the failure coming from looking for missing of_node or any other condition from parent/core driver. > > Another point, is that this reflects the real hardware. The same as we > > model entire SoC as multiple children of soc node (with their own > > properties), here we represent smaller chip which also has > > sub-components. > > Components we're calling things like "extcon"... I am rather thinking about charger, but yes, extcon as well. Either you have USB socket (and want to use associated logic) or not. Best regards, Krzysztof
Re: [PATCH] rtc: s5m: use devm_i2c_new_dummy_device()
On Tue, Jan 05, 2021 at 02:44:24PM +0100, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski
>
> Use the managed variant of i2c_new_dummy_device() to shrink code and
> remove the goto label.
>
> Signed-off-by: Bartosz Golaszewski
> ---
> drivers/rtc/rtc-s5m.c | 24
> 1 file changed, 8 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/rtc/rtc-s5m.c b/drivers/rtc/rtc-s5m.c
> index eb9dde4095a9..3432c6213b4c 100644
> --- a/drivers/rtc/rtc-s5m.c
> +++ b/drivers/rtc/rtc-s5m.c
> @@ -760,7 +760,8 @@ static int s5m_rtc_probe(struct platform_device *pdev)
> return -ENODEV;
> }
>
> - info->i2c = i2c_new_dummy_device(s5m87xx->i2c->adapter, RTC_I2C_ADDR);
> + info->i2c = devm_i2c_new_dummy_device(>dev, s5m87xx->i2c->adapter,
> + RTC_I2C_ADDR);
> if (IS_ERR(info->i2c)) {
> dev_err(>dev, "Failed to allocate I2C for RTC\n");
> return PTR_ERR(info->i2c);
> @@ -768,10 +769,9 @@ static int s5m_rtc_probe(struct platform_device *pdev)
>
> info->regmap = devm_regmap_init_i2c(info->i2c, regmap_cfg);
> if (IS_ERR(info->regmap)) {
> - ret = PTR_ERR(info->regmap);
> dev_err(>dev, "Failed to allocate RTC register map: %d\n",
> - ret);
> - goto err;
> + ret);
> + return PTR_ERR(info->regmap);
> }
>
> info->dev = >dev;
> @@ -781,10 +781,9 @@ static int s5m_rtc_probe(struct platform_device *pdev)
> if (s5m87xx->irq_data) {
> info->irq = regmap_irq_get_virq(s5m87xx->irq_data, alarm_irq);
> if (info->irq <= 0) {
> - ret = -EINVAL;
> dev_err(>dev, "Failed to get virtual IRQ %d\n",
> alarm_irq);
> - goto err;
> + return -EINVAL;
> }
> }
>
> @@ -797,10 +796,8 @@ static int s5m_rtc_probe(struct platform_device *pdev)
> info->rtc_dev = devm_rtc_device_register(>dev, "s5m-rtc",
>_rtc_ops, THIS_MODULE);
>
> - if (IS_ERR(info->rtc_dev)) {
> - ret = PTR_ERR(info->rtc_dev);
> - goto err;
> - }
> + if (IS_ERR(info->rtc_dev))
> + return PTR_ERR(info->rtc_dev);
>
> if (!info->irq) {
> dev_info(>dev, "Alarm IRQ not available\n");
> @@ -813,15 +810,10 @@ static int s5m_rtc_probe(struct platform_device *pdev)
> if (ret < 0) {
> dev_err(>dev, "Failed to request alarm IRQ: %d: %d\n",
> info->irq, ret);
> - goto err;
> + return ret;
> }
>
> return 0;
> -
> -err:
> - i2c_unregister_device(info->i2c);
> -
> - return ret;
> }
>
> static int s5m_rtc_remove(struct platform_device *pdev)
Unbind should OOPS now.
Best regards,
Krzysztof
Re: [PATCH v6 2/8] regulator: dt-bindings: Document max8997-pmic nodes
On Mon, Jan 04, 2021 at 06:27:34PM +, Mark Brown wrote: > On Mon, Jan 04, 2021 at 07:18:25PM +0100, Krzysztof Kozlowski wrote: > > On Mon, Jan 04, 2021 at 01:51:56PM +, Mark Brown wrote: > > > > > +- charger: Node for configuring the charger driver. > > > > + Required properties: > > > > + - compatible: "maxim,max8997-battery" > > > > > + Optional properties: > > > > + - extcon: extcon specifier for charging events > > > > + - charger-supply: regulator node for charging current > > > > > +- muic: Node used only by extcon consumers. > > > > + Required properties: > > > > + - compatible: "maxim,max8997-muic" > > > > Why do these need to appear in the DT binding? We know these features > > > are there simply from knowing this is a max8997. > > > If you refer to children nodes, then we do not know these entirely > > because the features could be disabled (pins not connected). In such > > case these subnodes can be disabled and MFD framework will not > > instantiate children devices. > > We can indicate the presence of features without adding new compatible > strings, that's just encoding the way Linux currently divides things up > into the bindings. For example having an extcon property seems like it > should be enough to figure out if we're using extcon. It won't be enough because MFD will create device for extcon and bind the driver. The same for the charger. We have a board where max8997 is used only as PMIC (providing regulators) without battery and USB connectivity. Another point, is that this reflects the real hardware. The same as we model entire SoC as multiple children of soc node (with their own properties), here we represent smaller chip which also has sub-components. Best regards, Krzysztof
Re: [PATCH v6 2/8] regulator: dt-bindings: Document max8997-pmic nodes
On Mon, Jan 04, 2021 at 01:51:56PM +, Mark Brown wrote: > On Wed, Dec 30, 2020 at 08:52:07PM +, Timon Baetz wrote: > > > +- charger: Node for configuring the charger driver. > > + Required properties: > > + - compatible: "maxim,max8997-battery" > > + Optional properties: > > + - extcon: extcon specifier for charging events > > + - charger-supply: regulator node for charging current > > + > > +- muic: Node used only by extcon consumers. > > + Required properties: > > + - compatible: "maxim,max8997-muic" > > Why do these need to appear in the DT binding? We know these features > are there simply from knowing this is a max8997. If you refer to children nodes, then we do not know these entirely because the features could be disabled (pins not connected). In such case these subnodes can be disabled and MFD framework will not instantiate children devices. If you mean "the properties" like extcon or charger, then indeed it's a good question. In theory, wires still could be routed differently, e.g. different charging regulator used as a charger. In practice this is highly unlikely, however such DT design allows easier hooking up of different devices and even potential re-usage of kernel drivers (also unlikely...). Best regards, Krzysztof
Re: [PATCH 25/31] memory: tegra30: convert to use devm_pm_opp_* API
On Sun, Jan 03, 2021 at 03:54:15AM +, Yangtao Li wrote: > Use devm_pm_opp_* API to simplify code. > > Signed-off-by: Yangtao Li > --- > drivers/memory/tegra/tegra30-emc.c | 29 + > 1 file changed, 9 insertions(+), 20 deletions(-) Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH 24/31] memory: tegra20: convert to use devm_pm_opp_* API
On Fri, Jan 01, 2021 at 04:55:00PM +, Yangtao Li wrote: > Use devm_pm_opp_* API to simplify code. > > Signed-off-by: Yangtao Li > --- > drivers/memory/tegra/tegra20-emc.c | 29 + > 1 file changed, 9 insertions(+), 20 deletions(-) > Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH 22/31] memory: samsung: exynos5422-dmc: fix return error in exynos5_init_freq_table
On Fri, Jan 01, 2021 at 04:54:58PM +, Yangtao Li wrote: > We can't always return -EINVAL, let's fix it. > > Signed-off-by: Yangtao Li > --- > drivers/memory/samsung/exynos5422-dmc.c | 10 +++--- > 1 file changed, 7 insertions(+), 3 deletions(-) Reviewed-by: Krzysztof Kozlowski I see that the next patch depends on it so feel free to take it via PM tree. Otherwise let me know. Best regards, Krzysztof
Re: [PATCH 23/31] memory: samsung: exynos5422-dmc: convert to use devm_pm_opp_* API
On Fri, Jan 01, 2021 at 04:54:59PM +, Yangtao Li wrote: > Use devm_pm_opp_* API to simplify code. > > Signed-off-by: Yangtao Li > --- > drivers/memory/samsung/exynos5422-dmc.c | 21 + > 1 file changed, 5 insertions(+), 16 deletions(-) Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH v5 2/2] arm64: dts: imx8mm: Add Gateworks i.MX 8M Mini Development Kits
On Mon, Jan 04, 2021 at 09:36:47AM -0800, Tim Harvey wrote: > The Gateworks Venice GW71xx-0x/GW72xx-0x/GW73xx-0x are development > kits consisting of a GW700x SoM and a Baseboard. Future SoM's such > as the GW701x will create additional combinations. > > The GW700x SoM contains: > - i.MX 8M Mini SoC > - LPDDR4 DRAM > - eMMC FLASH > - Gateworks System Controller (eeprom/pushbutton/reset/voltage-monitor) > - GbE PHY connected to the i.MX 8M Mini FEC > - Power Management IC > > The GW71xx Baseboard contains: > - 1x MiniPCIe Socket with USB2.0, PCIe, and SIM > - 1x RJ45 GbE (i.MX 8M Mini FEC) > - I/O connector with 1x-SPI/1x-I2C/1x-UART/4x-GPIO signals > - PCIe Clock generator > - GPS and accelerometer > - 1x USB 2.0 Front Panel connector > - wide range power supply > > The GW72xx Baseboard contains: > - 2x MiniPCIe Socket with USB2.0, PCIe, and SIM > - 2x RJ45 GbE (i.MX 8M Mini FEC and LAN743x) > - 1x MicroSD connector > - 1x USB 2.0 Front Panel connector > - 1x SPI connector > - 1x Serial connector supporting 2x-UART or 1x-UART configured as 1 of: >RS232 w/ flow-control, RS485, RS422 > - PCIe Clock generator > - GPS and accelerometer > - Media Expansion connector (MIPI-CSI/MIPI-DSI/GPIO/I2S) > - I/O connector with 2x-ADC,2x-GPIO,1x-UART,1x-I2C > - wide range power supply > > The GW73xx Baseboard contains: > - 3x MiniPCIe Socket with USB2.0, PCIe, and SIM > - 2x RJ45 GbE (i.MX 8M Mini FEC and LAN743x) > - 1x MicroSD connector > - 1x USB 2.0 Front Panel connector > - 1x SPI connector > - 1x Serial connector supporting 2x-UART or 1x-UART configured as 1 of: >RS232 w/ flow-control, RS485, RS422 > - WiFi/BT > - PCIe Clock generator > - GPS and accelerometer > - Media Expansion connector (MIPI-CSI/MIPI-DSI/GPIO/I2S) > - I/O connector with 2x-ADC,2x-GPIO,1x-UART,1x-I2C > - wide range power supply > > Signed-off-by: Tim Harvey > --- > v5: > - fixed typo/grammar in commit log (s/controll/control > s/comprised/consisting/) > - removed underscore from remaining node names > - removed blank line in hoggrp > - removed unused assigned-clocks and assigned-clock-rates node from usdhc > nodes > Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH v4 2/2] arm64: dts: imx8mm: Add Gateworks i.MX 8M Mini Development Kits
On Wed, Dec 30, 2020 at 09:58:43AM -0800, Tim Harvey wrote: > The Gateworks Venice GW71xx-0x/GW72xx-0x/GW73xx-0x are development > kits comprised of a GW700x SoM and a Baseboard. Future SoM's such > as the GW701x will create additional combinations. > > The GW700x SoM contains: > - i.MX 8M Mini SoC > - LPDDR4 DRAM > - eMMC FLASH > - Gateworks System Controller (eeprom/pushbutton/reset/voltage-monitor) > - GbE PHY connected to the i.MX 8M Mini FEC > - Power Management IC > > The GW71xx Baseboard contains: > - 1x MiniPCIe Socket with USB2.0, PCIe, and SIM > - 1x RJ45 GbE (i.MX 8M Mini FEC) > - I/O connector with 1x-SPI/1x-I2C/1x-UART/4x-GPIO signals > - PCIe Clock generator > - GPS and accelerometer > - 1x USB 2.0 Front Panel connector > - wide range power supply > > The GW72xx Baseboard contains: > - 2x MiniPCIe Socket with USB2.0, PCIe, and SIM > - 2x RJ45 GbE (i.MX 8M Mini FEC and LAN743x) > - 1x MicroSD connector > - 1x USB 2.0 Front Panel connector > - 1x SPI connector > - 1x Serial connector supporting 2x-UART or 1x-UART configured as 1 of: >RS232 w/ flow-controll, RS485, RS422 > - PCIe Clock generator > - GPS and accelerometer > - Media Expansion connector (MIPI-CSI/MIPI-DSI/GPIO/I2S) > - I/O connector with 2x-ADC,2x-GPIO,1x-UART,1x-I2C > - wide range power supply > > The GW73xx Baseboard contains: > - 3x MiniPCIe Socket with USB2.0, PCIe, and SIM > - 2x RJ45 GbE (i.MX 8M Mini FEC and LAN743x) > - 1x MicroSD connector > - 1x USB 2.0 Front Panel connector > - 1x SPI connector > - 1x Serial connector supporting 2x-UART or 1x-UART configured as 1 of: >RS232 w/ flow-controll, RS485, RS422 > - WiFi/BT > - PCIe Clock generator > - GPS and accelerometer > - Media Expansion connector (MIPI-CSI/MIPI-DSI/GPIO/I2S) > - I/O connector with 2x-ADC,2x-GPIO,1x-UART,1x-I2C > - wide range power supply > > Signed-off-by: Tim Harvey > --- > v4: > - replace underscore with hyphen for gpio-keys node > - add 'off-board header' comments to i2c/spi/uart nodes that go off-board > - move node comments to own line above node > - add spaces after comma > - move uart2_gpio rs485 config pinmux to hoggroup as they don't necessarily >relate to uart2 > - fix fifo-depth dt property for phy > > v3: > - fix gpio controller node name > - add rtc node to SoM > - add pmic pinctrl to SoM > - fixed compatible string for SoM eeprom's > > v2: > - fix i.MX 8M Mini name in commit log > - consistent use of underscore vs hyphen in labels > - fix gsc interrupt type > - fix iomux group node names > - fix led-controller bindings: >(use correct node names, color, function and remove label) > - use accelerometer node name vs accel > - remove sai3 from gw71xx baseboard > - added serial connector description to commit message > - added I/O connector description to commit message > - removed unnecessary #address-cells/#size-cells from gpio-keys node > > Signed-off-by: Tim Harvey > --- > arch/arm64/boot/dts/freescale/Makefile | 3 + > .../boot/dts/freescale/imx8mm-venice-gw700x.dtsi | 497 > + > .../boot/dts/freescale/imx8mm-venice-gw71xx-0x.dts | 19 + > .../boot/dts/freescale/imx8mm-venice-gw71xx.dtsi | 186 > .../boot/dts/freescale/imx8mm-venice-gw72xx-0x.dts | 20 + > .../boot/dts/freescale/imx8mm-venice-gw72xx.dtsi | 314 + > .../boot/dts/freescale/imx8mm-venice-gw73xx-0x.dts | 19 + > .../boot/dts/freescale/imx8mm-venice-gw73xx.dtsi | 366 +++ > 8 files changed, 1424 insertions(+) > create mode 100644 arch/arm64/boot/dts/freescale/imx8mm-venice-gw700x.dtsi > create mode 100644 arch/arm64/boot/dts/freescale/imx8mm-venice-gw71xx-0x.dts > create mode 100644 arch/arm64/boot/dts/freescale/imx8mm-venice-gw71xx.dtsi > create mode 100644 arch/arm64/boot/dts/freescale/imx8mm-venice-gw72xx-0x.dts > create mode 100644 arch/arm64/boot/dts/freescale/imx8mm-venice-gw72xx.dtsi > create mode 100644 arch/arm64/boot/dts/freescale/imx8mm-venice-gw73xx-0x.dts > create mode 100644 arch/arm64/boot/dts/freescale/imx8mm-venice-gw73xx.dtsi > > diff --git a/arch/arm64/boot/dts/freescale/Makefile > b/arch/arm64/boot/dts/freescale/Makefile > index f8d5943..ecdd233 100644 > --- a/arch/arm64/boot/dts/freescale/Makefile > +++ b/arch/arm64/boot/dts/freescale/Makefile > @@ -32,6 +32,9 @@ dtb-$(CONFIG_ARCH_MXC) += imx8mm-beacon-kit.dtb > dtb-$(CONFIG_ARCH_MXC) += imx8mm-evk.dtb > dtb-$(CONFIG_ARCH_MXC) += imx8mm-ddr4-evk.dtb > dtb-$(CONFIG_ARCH_MXC) += imx8mm-var-som-symphony.dtb > +dtb-$(CONFIG_ARCH_MXC) += imx8mm-venice-gw71xx-0x.dtb > +dtb-$(CONFIG_ARCH_MXC) += imx8mm-venice-gw72xx-0x.dtb > +dtb-$(CONFIG_ARCH_MXC) += imx8mm-venice-gw73xx-0x.dtb > dtb-$(CONFIG_ARCH_MXC) += imx8mn-evk.dtb > dtb-$(CONFIG_ARCH_MXC) += imx8mn-ddr4-evk.dtb > dtb-$(CONFIG_ARCH_MXC) += imx8mn-var-som-symphony.dtb > diff --git a/arch/arm64/boot/dts/freescale/imx8mm-venice-gw700x.dtsi >
Re: (subset) [PATCH -next] drivers: memory: Use DEFINE_SPINLOCK() for spinlock
On Mon, 28 Dec 2020 21:50:56 +0800, Zheng Yongjun wrote: > spinlock can be initialized automatically with DEFINE_SPINLOCK() > rather than explicitly calling spin_lock_init(). Applied, thanks! [1/1] drivers: memory: Use DEFINE_SPINLOCK() for spinlock commit: bd96a89ca3fe874c98fe057cccb087603d76e5d4 Best regards, -- Krzysztof Kozlowski
Re: (subset) [PATCH v6 1/8] extcon: max8997: Add CHGINS and CHGRM interrupt handling
On Wed, 30 Dec 2020 20:51:53 +, Timon Baetz wrote: > This allows the MAX8997 charger to set the current limit depending on > the detected extcon charger type. Applied, thanks! [8/8] ARM: dts: exynos: Add top-off charging regulator node for I9100 commit: 3803f461bd28c1c817281348509399778633e82f Best regards, -- Krzysztof Kozlowski
Re: (subset) [PATCH v6 1/8] extcon: max8997: Add CHGINS and CHGRM interrupt handling
On Wed, 30 Dec 2020 20:51:53 +, Timon Baetz wrote: > This allows the MAX8997 charger to set the current limit depending on > the detected extcon charger type. Applied, thanks! [7/8] ARM: dts: exynos: Fix charging regulator voltage and current for I9100 commit: 4a928b3b7c0f8a2ae382c3db3a78898877567786 Best regards, -- Krzysztof Kozlowski
Re: [PATCH v2 2/4] soc: samsung: exynos-asv: handle reading revision register error
On Mon, Dec 07, 2020 at 08:05:15PM +0100, Krzysztof Kozlowski wrote:
> If regmap_read() fails, the product_id local variable will contain
> random value from the stack. Do not try to parse such value and fail
> the ASV driver probe.
>
> Fixes: 5ea428595cc5 ("soc: samsung: Add Exynos Adaptive Supply Voltage
> driver")
> Cc:
> Signed-off-by: Krzysztof Kozlowski
> ---
> drivers/soc/samsung/exynos-asv.c | 8 +++-
Applied.
Best regards,
Krzysztof
Re: (subset) [PATCH v2 0/4] soc: samsung: exynos-chipid and asv improvements
On Mon, 7 Dec 2020 20:05:13 +0100, Krzysztof Kozlowski wrote: > Changes since v1: > 1. Drop patch "soc: samsung: exynos-chipid: initialize later - with >arch_initcall" which is now superseded by convertin to a driver. > 2. Include Marek's patch, just for the reference and rebase. > 3. Add patch "soc: samsung: exynos-asv: handle reading revision register >error". > 4. Add patch "soc: samsung: exynos-chipid: convert to driver and merge >exynos-asv". > > [...] Applied, thanks! [4/4] soc: samsung: exynos-chipid: convert to driver and merge exynos-asv commit: 352bfbb3e0230c96b2bce00d2ac3f0de303cc7b6 Best regards, -- Krzysztof Kozlowski
Re: [PATCH v2 1/4] soc: samsung: exynos-asv: don't defer early on not-supported SoCs
On Mon, Dec 07, 2020 at 08:05:14PM +0100, Krzysztof Kozlowski wrote:
> From: Marek Szyprowski
>
> Check if the SoC is really supported before gathering the needed
> resources. This fixes endless deferred probe on some SoCs other than
> Exynos5422 (like Exynos5410).
>
> Fixes: 5ea428595cc5 ("soc: samsung: Add Exynos Adaptive Supply Voltage
> driver")
> Cc:
> Signed-off-by: Marek Szyprowski
> Signed-off-by: Krzysztof Kozlowski
> ---
> drivers/soc/samsung/exynos-asv.c | 10 +-
Thanks, applied.
Best regards,
Krzysztof
Re: [PATCH v6 3/8] power: supply: max8997_charger: Set CHARGER current limit
On Wed, Dec 30, 2020 at 08:52:15PM +, Timon Baetz wrote: > Register for extcon notification and set charging current depending on > the detected cable type. Current values are taken from vendor kernel, > where most charger types end up setting 650mA [0]. > > Also enable and disable the CHARGER regulator based on extcon events. > > [0] > https://github.com/krzk/linux-vendor-backup/blob/samsung/galaxy-s2-epic-4g-touch-sph-d710-exynos4210-dump/drivers/misc/max8997-muic.c#L1675-L1678 > > Signed-off-by: Timon Baetz > --- > v6: dev_info() instead of dev_err(). > v5: Use devm_regulator_get_optional(), dev_err() on failure. > dev_err() on extcon_get_edev_by_phandle() failure. > v4: Make extcon and charger-supply optional. > v3: Split MFD change. > return on regulator_set_current_limit() failure. > v2: Split DTS changes. > Add missing include. > Rename charger_data members. > Disable regulator on regulator_set_current_limit() failure. > Fix ret declaration. > Remove unneeded variables. > Don't dev_err() on deferral. > Get regulator and extcon from DTS. > Use devm_regulator_get(). > Fix indentation. Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
[PATCH] extcon: Add stubs for extcon_register_notifier_all() functions
Add stubs for extcon_register_notifier_all() function for !CONFIG_EXTCON
case. This is useful for compile testing and for drivers which use
EXTCON but do not require it (therefore do not depend on CONFIG_EXTCON).
Fixes: 815429b39d94 ("extcon: Add new extcon_register_notifier_all() to monitor
all external connectors")
Reported-by: kernel test robot
Signed-off-by: Krzysztof Kozlowski
---
include/linux/extcon.h | 23 +++
1 file changed, 23 insertions(+)
diff --git a/include/linux/extcon.h b/include/linux/extcon.h
index fd183fb9c20f..0c19010da77f 100644
--- a/include/linux/extcon.h
+++ b/include/linux/extcon.h
@@ -271,6 +271,29 @@ static inline void devm_extcon_unregister_notifier(struct
device *dev,
struct extcon_dev *edev, unsigned int id,
struct notifier_block *nb) { }
+static inline int extcon_register_notifier_all(struct extcon_dev *edev,
+ struct notifier_block *nb)
+{
+ return 0;
+}
+
+static inline int extcon_unregister_notifier_all(struct extcon_dev *edev,
+struct notifier_block *nb)
+{
+ return 0;
+}
+
+static inline int devm_extcon_register_notifier_all(struct device *dev,
+ struct extcon_dev *edev,
+ struct notifier_block *nb)
+{
+ return 0;
+}
+
+static inline void devm_extcon_unregister_notifier_all(struct device *dev,
+ struct extcon_dev *edev,
+ struct notifier_block
*nb) { }
+
static inline struct extcon_dev *extcon_get_extcon_dev(const char *extcon_name)
{
return ERR_PTR(-ENODEV);
--
2.25.1
Re: [PATCH v6 3/8] power: supply: max8997_charger: Set CHARGER current limit
On Thu, Dec 31, 2020 at 07:19:07AM +, Timon Baetz wrote: > On Thu, 31 Dec 2020 07:22:22 +0800, kernel test robot wrote: > > Hi Timon, > > > > Thank you for the patch! Yet something to improve: > > > > [auto build test ERROR on regulator/for-next] > > [also build test ERROR on pinctrl-samsung/for-next krzk/for-next v5.11-rc1 > > next-20201223] > > [cannot apply to robh/for-next] > > [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] > > > > url: > > https://github.com/0day-ci/linux/commits/Timon-Baetz/extcon-max8997-Add-CHGINS-and-CHGRM-interrupt-handling/20201231-045812 > > base: > > https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git > > for-next > > config: arm-randconfig-r004-20201230 (attached as .config) > > compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project > > 3c0d36f977d9e012b245c796ddc8596ac3af659b) > > reproduce (this is a W=1 build): > > wget > > https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O > > ~/bin/make.cross > > chmod +x ~/bin/make.cross > > # install arm cross compiling tool for clang build > > # apt-get install binutils-arm-linux-gnueabi > > # > > https://github.com/0day-ci/linux/commit/3a597219bbfc1f9a0b65b9662b7b95bbb7cf728f > > git remote add linux-review https://github.com/0day-ci/linux > > git fetch --no-tags linux-review > > Timon-Baetz/extcon-max8997-Add-CHGINS-and-CHGRM-interrupt-handling/20201231-045812 > > git checkout 3a597219bbfc1f9a0b65b9662b7b95bbb7cf728f > > # save the attached .config to linux build tree > > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm > > > > If you fix the issue, kindly add following tag as appropriate > > Reported-by: kernel test robot > > > > All errors (new ones prefixed by >>): > > > > >> drivers/power/supply/max8997_charger.c:261:9: error: implicit > > >> declaration of function 'devm_extcon_register_notifier_all' > > >> [-Werror,-Wimplicit-function-declaration] > >ret = devm_extcon_register_notifier_all(>dev, > > charger->edev, > > ^ > >drivers/power/supply/max8997_charger.c:261:9: note: did you mean > > 'devm_extcon_register_notifier'? > >include/linux/extcon.h:263:19: note: 'devm_extcon_register_notifier' > > declared here > >static inline int devm_extcon_register_notifier(struct device *dev, > > ^ > >1 error generated. > > This is failing because CONFIG_EXTCON is not set and *_all() don't have > stub implementations in extcon.h. Should I add a fix for it in this > series? It is not problem of your patchset, so up to you. After your changes the driver still can work fine without CONFIG_EXTCON and CONFIG_REGULATOR. Best regards, Krzysztof
Re: [PATCH v5 1/8] extcon: max8997: Add CHGINS and CHGRM interrupt handling
On Mon, Dec 28, 2020 at 11:35:38AM +, Timon Baetz wrote: > This allows the MAX8997 charger to set the current limit depending on > the detected extcon charger type. > > Signed-off-by: Timon Baetz > --- > drivers/extcon/extcon-max8997.c | 4 > 1 file changed, 4 insertions(+) It's already a v5 but what are the changes here? You must provide the changelog for your patches - either in the cover letter or in each patch. Best regards, Krzysztof
Re: [PATCH v5 3/8] power: supply: max8997_charger: Set CHARGER current limit
On Mon, Dec 28, 2020 at 11:36:02AM +, Timon Baetz wrote:
> Register for extcon notification and set charging current depending on
> the detected cable type. Current values are taken from vendor kernel,
> where most charger types end up setting 650mA [0].
>
> Also enable and disable the CHARGER regulator based on extcon events.
>
> [0]
> https://github.com/krzk/linux-vendor-backup/blob/samsung/galaxy-s2-epic-4g-touch-sph-d710-exynos4210-dump/drivers/misc/max8997-muic.c#L1675-L1678
>
> Signed-off-by: Timon Baetz
> ---
> drivers/power/supply/max8997_charger.c | 96 ++
> 1 file changed, 96 insertions(+)
>
> diff --git a/drivers/power/supply/max8997_charger.c
> b/drivers/power/supply/max8997_charger.c
> index 1947af25879a..f0f725385dfc 100644
> --- a/drivers/power/supply/max8997_charger.c
> +++ b/drivers/power/supply/max8997_charger.c
> @@ -6,12 +6,14 @@
> // MyungJoo Ham
>
> #include
> +#include
> #include
> #include
> #include
> #include
> #include
> #include
> +#include
>
> /* MAX8997_REG_STATUS4 */
> #define DCINOK_SHIFT 1
> @@ -31,6 +33,10 @@ struct charger_data {
> struct device *dev;
> struct max8997_dev *iodev;
> struct power_supply *battery;
> + struct regulator *reg;
> + struct extcon_dev *edev;
> + struct notifier_block extcon_nb;
> + struct work_struct extcon_work;
> };
>
> static enum power_supply_property max8997_battery_props[] = {
> @@ -88,6 +94,67 @@ static int max8997_battery_get_property(struct
> power_supply *psy,
> return 0;
> }
>
> +static void max8997_battery_extcon_evt_stop_work(void *data)
> +{
> + struct charger_data *charger = data;
> +
> + cancel_work_sync(>extcon_work);
> +}
> +
> +static void max8997_battery_extcon_evt_worker(struct work_struct *work)
> +{
> + struct charger_data *charger =
> + container_of(work, struct charger_data, extcon_work);
> + struct extcon_dev *edev = charger->edev;
> + int current_limit;
> +
> + if (extcon_get_state(edev, EXTCON_CHG_USB_SDP) > 0) {
> + dev_dbg(charger->dev, "USB SDP charger is connected\n");
> + current_limit = 45;
> + } else if (extcon_get_state(edev, EXTCON_CHG_USB_DCP) > 0) {
> + dev_dbg(charger->dev, "USB DCP charger is connected\n");
> + current_limit = 65;
> + } else if (extcon_get_state(edev, EXTCON_CHG_USB_FAST) > 0) {
> + dev_dbg(charger->dev, "USB FAST charger is connected\n");
> + current_limit = 65;
> + } else if (extcon_get_state(edev, EXTCON_CHG_USB_SLOW) > 0) {
> + dev_dbg(charger->dev, "USB SLOW charger is connected\n");
> + current_limit = 65;
> + } else if (extcon_get_state(edev, EXTCON_CHG_USB_CDP) > 0) {
> + dev_dbg(charger->dev, "USB CDP charger is connected\n");
> + current_limit = 65;
> + } else {
> + dev_dbg(charger->dev, "USB charger is diconnected\n");
> + current_limit = -1;
> + }
> +
> + if (current_limit > 0) {
> + int ret = regulator_set_current_limit(charger->reg,
> current_limit, current_limit);
> +
> + if (ret) {
> + dev_err(charger->dev, "failed to set current limit:
> %d\n", ret);
> + return;
> + }
> + ret = regulator_enable(charger->reg);
> + if (ret)
> + dev_err(charger->dev, "failed to enable regulator:
> %d\n", ret);
> + } else {
> + int ret = regulator_disable(charger->reg);
> +
> + if (ret)
> + dev_err(charger->dev, "failed to disable regulator:
> %d\n", ret);
> + }
> +}
> +
> +static int max8997_battery_extcon_evt(struct notifier_block *nb,
> + unsigned long event, void *param)
> +{
> + struct charger_data *charger =
> + container_of(nb, struct charger_data, extcon_nb);
> + schedule_work(>extcon_work);
> + return NOTIFY_OK;
> +}
> +
> static const struct power_supply_desc max8997_battery_desc = {
> .name = "max8997_pmic",
> .type = POWER_SUPPLY_TYPE_BATTERY,
> @@ -170,6 +237,35 @@ static int max8997_battery_probe(struct platform_device
> *pdev)
> return PTR_ERR(charger->battery);
> }
>
> + charger->reg = devm_regulator_get_optional(>dev, "charger");
> + if (IS_ERR(charger->reg)) {
> + if (PTR_ERR(charger->reg) == -EPROBE_DEFER)
> + return -EPROBE_DEFER;
> + dev_err(>dev, "couldn't get charger regulator\n");
This should be dev_info, as we discussed. Otherwise it will scream on
every boot with a DTS which does not have charger node.
Best regards,
Krzysztof
Re: [PATCH v3 2/2] arm64: dts: imx8mm: Add Gateworks i.MX 8M Mini Development Kits
On Mon, Dec 28, 2020 at 04:21:17PM -0800, Tim Harvey wrote:
> The Gateworks Venice GW71xx-0x/GW72xx-0x/GW73xx-0x are development
> kits comprised of a GW700x SoM and a Baseboard. Future SoM's such
> as the GW701x will create additional combinations.
>
> The GW700x SoM contains:
> - i.MX 8M Mini SoC
> - LPDDR4 DRAM
> - eMMC FLASH
> - Gateworks System Controller (eeprom/pushbutton/reset/voltage-monitor)
> - GbE PHY connected to the i.MX 8M Mini FEC
> - Power Management IC
>
> The GW71xx Baseboard contains:
> - 1x MiniPCIe Socket with USB2.0, PCIe, and SIM
> - 1x RJ45 GbE (i.MX 8M Mini FEC)
> - I/O connector with 1x-SPI/1x-I2C/1x-UART/4x-GPIO signals
> - PCIe Clock generator
> - GPS and accelerometer
> - 1x USB 2.0 Front Panel connector
> - wide range power supply
>
> The GW72xx Baseboard contains:
> - 2x MiniPCIe Socket with USB2.0, PCIe, and SIM
> - 2x RJ45 GbE (i.MX 8M Mini FEC and LAN743x)
> - 1x MicroSD connector
> - 1x USB 2.0 Front Panel connector
> - 1x SPI connector
> - 1x Serial connector supporting 2x-UART or 1x-UART configured as 1 of:
>RS232 w/ flow-controll, RS485, RS422
> - PCIe Clock generator
> - GPS and accelerometer
> - Media Expansion connector (MIPI-CSI/MIPI-DSI/GPIO/I2S)
> - I/O connector with 2x-ADC,2x-GPIO,1x-UART,1x-I2C
> - wide range power supply
>
> The GW73xx Baseboard contains:
> - 3x MiniPCIe Socket with USB2.0, PCIe, and SIM
> - 2x RJ45 GbE (i.MX 8M Mini FEC and LAN743x)
> - 1x MicroSD connector
> - 1x USB 2.0 Front Panel connector
> - 1x SPI connector
> - 1x Serial connector supporting 2x-UART or 1x-UART configured as 1 of:
>RS232 w/ flow-controll, RS485, RS422
> - WiFi/BT
> - PCIe Clock generator
> - GPS and accelerometer
> - Media Expansion connector (MIPI-CSI/MIPI-DSI/GPIO/I2S)
> - I/O connector with 2x-ADC,2x-GPIO,1x-UART,1x-I2C
> - wide range power supply
>
> Signed-off-by: Tim Harvey
> ---
> v3:
> - fix gpio controller node name
> - add rtc node to SoM
> - add pmic pinctrl to SoM
> - fixed compatible string for SoM eeprom's
>
> v2:
> - fix i.MX 8M Mini name in commit log
> - consistent use of underscore vs hyphen in labels
> - fix gsc interrupt type
> - fix iomux group node names
> - fix led-controller bindings:
>(use correct node names, color, function and remove label)
> - use accelerometer node name vs accel
> - remove sai3 from gw71xx baseboard
> - added serial connector description to commit message
> - added I/O connector description to commit message
> - removed unnecessary #address-cells/#size-cells from gpio-keys node
> ---
> arch/arm64/boot/dts/freescale/Makefile | 3 +
> .../boot/dts/freescale/imx8mm-venice-gw700x.dtsi | 495
> +
> .../boot/dts/freescale/imx8mm-venice-gw71xx-0x.dts | 19 +
> .../boot/dts/freescale/imx8mm-venice-gw71xx.dtsi | 182
> .../boot/dts/freescale/imx8mm-venice-gw72xx-0x.dts | 20 +
> .../boot/dts/freescale/imx8mm-venice-gw72xx.dtsi | 316 +
> .../boot/dts/freescale/imx8mm-venice-gw73xx-0x.dts | 19 +
> .../boot/dts/freescale/imx8mm-venice-gw73xx.dtsi | 368 +++
> 8 files changed, 1422 insertions(+)
> create mode 100644 arch/arm64/boot/dts/freescale/imx8mm-venice-gw700x.dtsi
> create mode 100644 arch/arm64/boot/dts/freescale/imx8mm-venice-gw71xx-0x.dts
> create mode 100644 arch/arm64/boot/dts/freescale/imx8mm-venice-gw71xx.dtsi
> create mode 100644 arch/arm64/boot/dts/freescale/imx8mm-venice-gw72xx-0x.dts
> create mode 100644 arch/arm64/boot/dts/freescale/imx8mm-venice-gw72xx.dtsi
> create mode 100644 arch/arm64/boot/dts/freescale/imx8mm-venice-gw73xx-0x.dts
> create mode 100644 arch/arm64/boot/dts/freescale/imx8mm-venice-gw73xx.dtsi
>
> diff --git a/arch/arm64/boot/dts/freescale/Makefile
> b/arch/arm64/boot/dts/freescale/Makefile
> index f8d5943..ecdd233 100644
> --- a/arch/arm64/boot/dts/freescale/Makefile
> +++ b/arch/arm64/boot/dts/freescale/Makefile
> @@ -32,6 +32,9 @@ dtb-$(CONFIG_ARCH_MXC) += imx8mm-beacon-kit.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mm-evk.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mm-ddr4-evk.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mm-var-som-symphony.dtb
> +dtb-$(CONFIG_ARCH_MXC) += imx8mm-venice-gw71xx-0x.dtb
> +dtb-$(CONFIG_ARCH_MXC) += imx8mm-venice-gw72xx-0x.dtb
> +dtb-$(CONFIG_ARCH_MXC) += imx8mm-venice-gw73xx-0x.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mn-evk.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mn-ddr4-evk.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mn-var-som-symphony.dtb
> diff --git a/arch/arm64/boot/dts/freescale/imx8mm-venice-gw700x.dtsi
> b/arch/arm64/boot/dts/freescale/imx8mm-venice-gw700x.dtsi
> new file mode 100644
> index ..bc63525
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx8mm-venice-gw700x.dtsi
> @@ -0,0 +1,495 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright 2020 Gateworks Corporation
> + */
> +
> +#include
> +#include
> +#include
> +
> +/ {
> + memory@4000 {
> + device_type = "memory";
>
Re: [PATCH 3/4] arm64: dts: imx8mn: add spba bus node
On Tue, Dec 29, 2020 at 08:00:44PM +0800, peng@nxp.com wrote:
> From: Peng Fan
>
> According to RM, there is a spba bus inside aips3 and aips1, add it.
This does not look like matching contents of commit.
Best regards,
Krzysztof
>
> Signed-off-by: Peng Fan
> ---
> arch/arm64/boot/dts/freescale/imx8mn.dtsi | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> index 73602832ccaa..033fa90570ff 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> @@ -749,7 +749,7 @@ uart2: serial@3089 {
> clock-names = "ipg", "per";
> status = "disabled";
> };
> - }
> + };
>
> crypto: crypto@3090 {
> compatible = "fsl,sec-v4.0";
> --
> 2.28.0
>
Re: [PATCH 2/4] arm64: dts: imx8mn: add spba bus node
On Tue, Dec 29, 2020 at 06:26:41AM -0600, Adam Ford wrote:
> On Tue, Dec 29, 2020 at 6:15 AM wrote:
> >
> > From: Peng Fan
> >
> > According to RM, there is a spba bus inside aips3 and aips1, add it.
> >
> > Signed-off-by: Peng Fan
> > ---
> > arch/arm64/boot/dts/freescale/imx8mm.dtsi | 362 +++---
> > 1 file changed, 189 insertions(+), 173 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> > b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> > index c824f2615fe8..91f85b8cee9a 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> > @@ -269,117 +269,125 @@ aips1: bus@3000 {
> > #size-cells = <1>;
> > ranges = <0x3000 0x3000 0x40>;
> >
> > - sai1: sai@3001 {
> > - #sound-dai-cells = <0>;
> > - compatible = "fsl,imx8mm-sai",
> > "fsl,imx8mq-sai";
> > - reg = <0x3001 0x1>;
> > - interrupts = > IRQ_TYPE_LEVEL_HIGH>;
> > - clocks = < IMX8MM_CLK_SAI1_IPG>,
> > -< IMX8MM_CLK_SAI1_ROOT>,
> > -< IMX8MM_CLK_DUMMY>, <
> > IMX8MM_CLK_DUMMY>;
> > - clock-names = "bus", "mclk1", "mclk2",
> > "mclk3";
> > - dmas = < 0 2 0>, < 1 2 0>;
> > - dma-names = "rx", "tx";
> > - status = "disabled";
> > - };
> > + bus@3000 {
>
> There is already a bus@3000 (aips1), and I think the system
> doesn't like it when there are multiple busses with the same name.
>
> There was some discussion on fixing the 8mn [1], but it doesn't look
> like it went anywhere.
>
> I am guessing the Mini will need something similar to the nano.
>
> [1] -
> https://patchwork.kernel.org/project/linux-arm-kernel/patch/1607324004-12960-1-git-send-email-shengjiu.w...@nxp.com/
Several replies from S.j. Wang are missing from LKML (and maybe
patchwork?) but we reached a conclusion:
https://lore.kernel.org/linux-arm-kernel/20201208090601.GA8347@kozik-lap/
Either you do some remapping of address space or just rename the "bus"
nodes (e.g. generic bus-1 or a specific spba-bus).
Best regards,
Krzysztof
Re: [PATCH 1/9] ARM: dts: exynos: correct PMIC interrupt trigger level on Artik 5
On Thu, 10 Dec 2020 22:28:55 +0100, Krzysztof Kozlowski wrote: > The Samsung PMIC datasheets describe the interrupt line as active low > with a requirement of acknowledge from the CPU. Without specifying the > interrupt type in Devicetree, kernel might apply some fixed > configuration, not necessarily working for this hardware. Applied, thanks! [1/9] ARM: dts: exynos: correct PMIC interrupt trigger level on Artik 5 commit: 58139a7837133538099dc59447f33765b61f5c27 [2/9] ARM: dts: exynos: correct PMIC interrupt trigger level on Monk commit: c9b260c91ab61d4094a3c152546d88d41259d647 [3/9] ARM: dts: exynos: correct PMIC interrupt trigger level on Rinato commit: 2c9f52d2b09abd25bd00ef2a5a35d9246fc92d88 [4/9] ARM: dts: exynos: correct PMIC interrupt trigger level on Spring commit: 1d6f6eee94da4f60ddb2107ffcf55629083711df [5/9] ARM: dts: exynos: correct PMIC interrupt trigger level on Arndale Octa commit: 4a96ea5cf0550766397f5e9221c4f2a949492ee6 [6/9] ARM: dts: exynos: correct PMIC interrupt trigger level on Odroid XU3 family commit: 0274326ce6796813842998141174bd5a0e9ff908 [7/9] arm64: dts: exynos: correct PMIC interrupt trigger level on TM2 commit: 9fd8f10d119c6c48899ace33ff0f7e8702ad1d66 [8/9] arm64: dts: exynos: correct PMIC interrupt trigger level on Espresso commit: acdd83e384c41d20d66bc0045f5eb67b6d67ed69 Best regards, -- Krzysztof Kozlowski
Re: (subset) [PATCH] arm64: dts: exynos: correct S3FWRN5 NFC interrupt trigger level on TM2
On Thu, 10 Dec 2020 22:18:58 +0100, Krzysztof Kozlowski wrote: > The S3FWRN5 datasheet describe the interrupt line as rising edge. The > current configuration as level high, could cause spurious interrupts. Applied, thanks! [1/1] arm64: dts: exynos: correct S3FWRN5 NFC interrupt trigger level on TM2 commit: d497d07b74f05f01b9c8c85b6679c2df57ecbdb4 Best regards, -- Krzysztof Kozlowski
Re: (subset) [PATCH 0/2] Fix USB2 PHY operation on Exynos542x
On Fri, 20 Nov 2020 09:56:35 +0100, Marek Szyprowski wrote: > This patchset finally fixes the last remaining issue related to the > system suspend/resume on Odroid XU3/XU4/HC1 board family. It can be > observed that system suspend/resume fails on XU4/HC1 when kernel is > compiled from multi_v7_defconfig. Chaning the configuration a bit - > switching Exynos USB2 PHY driver to be built-in surprisingly fixed that > issue. An investigation revealed that the Exynos USB2 PHY driver poked > wrong registers in the PMU area on Exynos5420 SoCs breaking the USB3.0 > DRD PHY operation, what caused the suspend failure. Fix this by learning > the Exynos USB2 PHY driver about the Exynos5420 variant. > > [...] Applied, thanks! [2/2] ARM: dts: exynos: use Exynos5420 dedicated USB2 PHY compatible commit: 75681980c4e3d89c55b5b8f20b8f4c1aace601be Best regards, -- Krzysztof Kozlowski
Re: [PATCH 03/18] ARM: dts: exynos: correct fuel gauge interrupt trigger level on Midas family
On Thu, Dec 10, 2020 at 10:25:19PM +0100, Krzysztof Kozlowski wrote:
> The Maxim fuel gauge datasheets describe the interrupt line as active
> low with a requirement of acknowledge from the CPU. The falling edge
> interrupt will mostly work but it's not correct.
>
> Fixes: e8614292cd41 ("ARM: dts: Add Maxim 77693 fuel gauge node for
> exynos4412-trats2")
> Signed-off-by: Krzysztof Kozlowski
> ---
> arch/arm/boot/dts/exynos4412-midas.dtsi | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Hi Marek,
I remember you were reporting that on Trats2 you see fuel gauge
interrupt storm with this patchset. Is it correct? Shall I wait with
applying this?
Best regards,
Krzysztof
> diff --git a/arch/arm/boot/dts/exynos4412-midas.dtsi
> b/arch/arm/boot/dts/exynos4412-midas.dtsi
> index 111c32bae02c..b8b75dc81aa1 100644
> --- a/arch/arm/boot/dts/exynos4412-midas.dtsi
> +++ b/arch/arm/boot/dts/exynos4412-midas.dtsi
> @@ -221,7 +221,7 @@ i2c_max77693_fuel: i2c-gpio-3 {
> fuel-gauge@36 {
> compatible = "maxim,max17047";
> interrupt-parent = <>;
> - interrupts = <3 IRQ_TYPE_EDGE_FALLING>;
> + interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
> pinctrl-names = "default";
> pinctrl-0 = <_fuel_irq>;
> reg = <0x36>;
> --
> 2.25.1
>
Re: [PATCH 2/2] ARM: dts: exynos: use Exynos5420 dedicated USB2 PHY compatible
On Fri, Nov 20, 2020 at 12:07:44PM +0100, Marek Szyprowski wrote:
> Hi Krzysztof,
>
> On 20.11.2020 12:05, Krzysztof Kozlowski wrote:
> > On Fri, Nov 20, 2020 at 09:56:37AM +0100, Marek Szyprowski wrote:
> >> USB2.0 PHY in Exynos5420 differs from Exynos5250 variant a bit, so use the
> >> recently introduced dedicated compatible for Exynos5420.
> >>
> >> Signed-off-by: Marek Szyprowski
> >> ---
> >> arch/arm/boot/dts/exynos54xx.dtsi | 6 +++---
> >> 1 file changed, 3 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/arch/arm/boot/dts/exynos54xx.dtsi
> >> b/arch/arm/boot/dts/exynos54xx.dtsi
> >> index fe9d34c23374..2ddb7a5f12b3 100644
> >> --- a/arch/arm/boot/dts/exynos54xx.dtsi
> >> +++ b/arch/arm/boot/dts/exynos54xx.dtsi
> >> @@ -188,7 +188,7 @@
> >>compatible = "samsung,exynos4210-ehci";
> >>reg = <0x1211 0x100>;
> >>interrupts = ;
> >> - phys = <_phy 1>;
> >> + phys = <_phy 0>;
> >>phy-names = "host";
> >>};
> >>
> >> @@ -196,12 +196,12 @@
> >>compatible = "samsung,exynos4210-ohci";
> >>reg = <0x1212 0x100>;
> >>interrupts = ;
> >> - phys = <_phy 1>;
> >> + phys = <_phy 0>;
> >>phy-names = "host";
> >>};
> >>
> >>usb2_phy: phy@1213 {
> >> - compatible = "samsung,exynos5250-usb2-phy";
> >> + compatible = "samsung,exynos5420-usb2-phy";
> > The DTS change will wait till PHY driver adjustements get merged... or
> > if the difference is not critical, maybe using both compatibles (5420
> > and 5250) would have sense?
>
> It won't work easily with both compatibles, because in the 5420 variant
> I've also changed the PHY indices (5420 has no device and second hsic
> phy). IMHO the dts change can wait for the next release.
Thanks, applied.
Best regards,
Krzysztof
Re: [PATCH 2/2] arm64: dts: imx8mm: Add Gateworks IMX8MM Development Kits
On Mon, Dec 28, 2020 at 11:02:40AM -0800, Tim Harvey wrote:
> > +
> > > + {
> > > + clock-frequency = <10>;
> > > + pinctrl-names = "default";
> > > + pinctrl-0 = <_i2c1>;
> > > + status = "okay";
> > > +
> > > + gsc: gsc@20 {
> >
> > Node name should describe class of a device so maybe
> > "system-controller"?
> >
>
> The node-name here must match 'gsc' due to the bindings. The name
> stands for 'Gateworks System Controller'.
Indeed, so it must stay like this.
> > +
> > > + fan-controller@0 {
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> >
> > These do not seem needed.
>
> They are required per
> Documentation/devicetree/bindings/mfd/gateworks-gsc.yaml but they
> should be inherited from the parent node right? So perhaps
> gateworks-gsc.yaml needs to remove them?
Maybe they shouldn't be in the bindings, either? These are only for
nodes which have childre. The fan-controller does not have them.
>
> >
> > > + compatible = "gw,gsc-fan";
> > > + reg = <0x0a>;
> > > + };
> > > + };
> > > +
> > > + gsc_gpio: pca9555@23 {
> >
> > Lookes like gpio-controller, so "gpio". Please run make dtbs_check to be
> > sure your board validate against dt-schema.
>
> Yes, I see these warnings now with 'make dtbs_check W=1'
dtbs_check without W=1.
dtbs with W=1.
These are different tools.
>
> Note when I do run 'make dtbs_check W=1' I see many warnings which
> seem invalid that I have to skip over:
> /usr/src/linux/arch/arm64/boot/dts/freescale/imx8mm-venice-gw71xx-0x.dt.yaml:
> pi
> nctrl@3033: fec1grp:fsl,pins:0: [104, 720, 0, 0, 0, 3, 108, 724, 1216, 0,
> 1,
> 3, 112, 728, 0, 0, 0, 31, 116, 732, 0, 0, 0, 31, 120, 736, 0, 0, 0,
> 31, 124, 740, 0, 0, 0, 31, 156, 772, 0, 0, 0, 145, 152, 768, 0, 0, 0,
> 145, 148, 764, 0, 0, 0, 145, 144, 760, 0, 0, 0, 145, 132, 748, 0, 0,
> 0, 31, 140, 756, 0, 0, 0, 145, 136, 752, 0, 0, 0, 145, 128, 744, 0, 0,
> 0, 31, 244, 860, 0, 5, 0, 25] is too long
> From schema:
> /usr/src/linux/Documentation/devicetree/bindings/pinctrl/fsl,imx8mm-pinctrl.yaml
> /usr/src/linux/arch/arm64/boot/dts/freescale/imx8mm-venice-gw71xx-0x.dt.yaml:
> pinctrl@3033: fec1grp:fsl,pins:0: Additional items are not allowed
> (108, 724, 1216, 0, 1, 3, 112, 728, 0, 0, 0, 31, 116, 732, 0, 0, 0,
> 31, 120, 736, 0, 0, 0, 31, 124, 740, 0, 0, 0, 31, 156, 772, 0, 0, 0,
> 145, 152, 768, 0, 0, 0, 145, 148, 764, 0, 0, 0, 145, 144, 760, 0, 0,
> 0, 145, 132, 748, 0, 0, 0, 31, 140, 756, 0, 0, 0, 145, 136, 752, 0, 0,
> 0, 145, 128, 744, 0, 0, 0, 31, 244, 860, 0, 5, 0, 25 were unexpected)
> ^^^ It seems maybe something is too restrictive with
> Documentation/devicetree/bindings/pinctrl/fsl,imx8mm-pinctrl.yaml that
> doesn't allow an array of pins?
This is expected because dt-schema does not accept pin groups in one
array (<..., >). Compare your warnings with some other recent imx8mm
designs. If you introduce new, these have to be fixed.
>
> I see lots of warnings about imx8mm gpmi as well but they don't have
> anything to do with my dtbs.
>
> >
> > > + compatible = "nxp,pca9555";
> > > + reg = <0x23>;
> > > + gpio-controller;
> > > + #gpio-cells = <2>;
> > > + interrupt-parent = <>;
> > > + interrupts = <4>;
> > > + };
> > > +
>
> > > +
> > > + {
> > > + pinctrl_fec1: fec1grp {
> > > + fsl,pins = <
> > > + MX8MM_IOMUXC_ENET_MDC_ENET1_MDC 0x3
> > > + MX8MM_IOMUXC_ENET_MDIO_ENET1_MDIO 0x3
> > > + MX8MM_IOMUXC_ENET_TD3_ENET1_RGMII_TD3 0x1f
> > > + MX8MM_IOMUXC_ENET_TD2_ENET1_RGMII_TD2 0x1f
> > > + MX8MM_IOMUXC_ENET_TD1_ENET1_RGMII_TD1 0x1f
> > > + MX8MM_IOMUXC_ENET_TD0_ENET1_RGMII_TD0 0x1f
> > > + MX8MM_IOMUXC_ENET_RD3_ENET1_RGMII_RD3 0x91
> > > + MX8MM_IOMUXC_ENET_RD2_ENET1_RGMII_RD2 0x91
> > > + MX8MM_IOMUXC_ENET_RD1_ENET1_RGMII_RD1 0x91
> > > + MX8MM_IOMUXC_ENET_RD0_ENET1_RGMII_RD0 0x91
> > > + MX8MM_IOMUXC_ENET_TXC_ENET1_RGMII_TXC 0x1f
> > > + MX8MM_IOMUXC_ENET_RXC_ENET1_RGMII_RXC 0x91
> > > + MX8MM_IOMUXC_ENET_RX_CTL_ENET1_RGMII_RX_CTL 0x91
> > > + MX8MM_IOMUXC_ENET_TX_CTL_ENET1_RGMII_TX_CTL 0x1f
> > > + MX8MM_IOMUXC_NAND_ALE_GPIO3_IO0 0x19
> > > + >;
> > > + };
> > > +
> > > + pinctrl_gsc: gscirq {
> >
> > grp suffix. I think dtbs_check should point this out.
>
> ok, I do see this warning with 'make dtbs_check W=1'. I will fix the
> node names for the iomux pin groups
>
>
> >
> > > diff --git
Re: [PATCH v4 4/7] power: supply: max8997_charger: Set CHARGER current limit
On Fri, Dec 25, 2020 at 11:33:21AM +, Timon Baetz wrote: > On Thu, 24 Dec 2020 15:00:38 +0100, Krzysztof Kozlowski wrote: > > On Thu, Dec 24, 2020 at 02:37:06PM +0100, Krzysztof Kozlowski wrote: > > > On Thu, Dec 24, 2020 at 01:13:02PM +, Timon Baetz wrote: > > > > On Thu, 24 Dec 2020 10:55:59 +0100, Krzysztof Kozlowski wrote: > > > > > > @@ -170,6 +237,28 @@ static int max8997_battery_probe(struct > > > > > > platform_device *pdev) > > > > > > return PTR_ERR(charger->battery); > > > > > > } > > > > > > > > > > > > + charger->reg = devm_regulator_get(>dev, "charger"); > > > > > > > > > > Since you do not use get_optional, you will always get a dummy > > > > > regulator. In case of error, you should either print it or entirely > > > > > fail > > > > > the probe. Silently continuing makes it difficult to spot errors. > > > > > > > > > > Since the driver could operate in case of extcon/regulator error, just > > > > > dev_err() so failure will be spotted with dmesg. > > > > > > > > I will switch to devm_regulator_get_optional() and print an error on > > > > failure, thanks. > > > > > > > > > It will complain on older DTBs because you are introducing > > > > > incompatible > > > > > change, but that's expected. Just correct all other in-tree DTS. > > > > > > > > The other 2 in-tree DTS don't have CHARGER regulators. Not sure > > > > how to correct those. Should I add muic and charger nodes without a > > > > charger-supply? It will still complain in that case. > > > > > > +Cc Marek, > > > > > > This is why leaving the code as is - devm_regulator_get(), not optional > > > - makes sense. Core would provide dummy regulator, so you only have to > > > provide MUIC node. > > > > > > If you change the code to devm_regulator_get_optional(), you need to add > > > everything: the charger regulator, the charger node and MUIC node. > > > > > > For Trats, the configuration should be similar as i9100, although I > > > don't know the exact values of chargign voltage. > > > > > > For Origen, there is no battery, so the power supply should not bind. > > > Maybe this could be achieved with "status disabled" for charger node? It > > > depends whether MFD will respect such field... If it disables the > > > charger, you're done. > > > > I just looked at the MFD code and tested it - it nicely skips disabled > > devices. Therefore, for Origen I propose to add disabled nodes for > > charger and MUIC because these pins are not connected. No need to add > > regulators in such case. > > With a dummy regulator regulator_set_current_limit() fails with -EINVAL. Good point. > Isn't it better to just skip charging control (and dev_info()) when there > is no extcon or regulator? The charger driver would still probe > without those 2 properties and work as before. Yes, makes sense. > > Adding disabled nodes for Origen would probably still makes sense. > > I also noticed that adding nodes for those MFD cells prints "DMA mask > not set" which seems to be related to https://lkml.org/lkml/2020/4/23/873. > Any suggestions on how to handle that? I don't think it is your problem to solve. It affects other MFD devices as well. Best regards, Krzysztof
Re: [PATCH v3 4/6] arm64: dts: imx8mm: Add Engicam i.Core MX8M Mini C.TOUCH 2.0
On Mon, 28 Dec 2020 at 09:21, Jagan Teki wrote: > > > #include "imx8mm.dtsi" > > > #include "imx8mm-beacon-som.dtsi" > > > #include "imx8mm-beacon-baseboard.dtsi" > > > > > > (SoC dtsi, SoM dtsi, Carrier board dtsi) > > > > > > > design which makes any sense. We do not create empty DTS files which > > > > only include one more DTSI. The contents of > > > > imx8mm-engicam-ctouch2.dtsi should be directly in > > > > imx8mm-icore-mx8mm-ctouch2.dts. That's the same problem as with v1 - > > > > you overcomplicate simple stuff. It really looks like you ignored the > > > > comments from v1 in multiple places. > > > > > > As explained above, the design is pretty much the same as the existing > > > SoM's. > > > > > > imx8mm-engicam-ctouch2.dtsi is not just a dtsi file where nodes are > > > enabled. It has nodes enabled for Carrier board, so keeping nodes > > > separately will > > > > The files represent real devices or their components. So you have a > > SOM - a DTSI file. You have a carrier board - a DTS file. That's > > simple design which is mostly followed, unless something over > > complicated passes the review. > > > > > 1. More verbose for which IP's are available in the carrier board > > > > No difference when carrier DTSI is the DTS. Exactly the same. > > > > > 2. Easy to extend if someone can create another SoM with a similar > > > Carrier. > > > > Not really, if they include carrier DTSI they need to override a lot. > > So usually (including practice - I did it) they *copy* the carrier to > > create their own design. > > But what if the new board has slite change to use exiting carrier like > what ctouch2 10" OF. Can we add ctouch2 dtsi as a separate file for > this case? If you submit another DTS using the imx8mm-engicam-ctouch2.dtsi - with its own differences of course (not copying other DTS...) - then having a DTSI makes sense. In current form, still NAK for all the reasons I explained more than once. Best regards, Krzysztof
[PATCH] ath9k: Add separate entry for LED triggers to fix module builds
After commit 72cdab808714 ("ath9k: Do not select MAC80211_LEDS by
default") a configuration like:
- MAC80211_LEDS=y
- LEDS_CLASS=m
- NEW_LEDS=y
- ATH9K=y
leads to a build failure:
/usr/bin/ld: drivers/net/wireless/ath/ath9k/gpio.o: in function
`ath_deinit_leds':
drivers/net/wireless/ath/ath9k/gpio.c:69: undefined reference to
`led_classdev_unregister'
/usr/bin/ld: drivers/net/wireless/ath/ath9k/gpio.o: in function
`led_classdev_register':
include/linux/leds.h:190: undefined reference to `led_classdev_register_ext'
To be able to use LED triggers, the LEDS_CLASS can only be a module
if ath9k driver is a module as well.
Reported-by: kernel test robot
Fixes: 72cdab808714 ("ath9k: Do not select MAC80211_LEDS by default")
Signed-off-by: Krzysztof Kozlowski
---
drivers/net/wireless/ath/ath9k/Kconfig| 18 --
drivers/net/wireless/ath/ath9k/ath9k.h| 4 ++--
drivers/net/wireless/ath/ath9k/gpio.c | 2 +-
drivers/net/wireless/ath/ath9k/htc.h | 6 +++---
drivers/net/wireless/ath/ath9k/htc_drv_gpio.c | 2 +-
drivers/net/wireless/ath/ath9k/htc_drv_init.c | 4 ++--
drivers/net/wireless/ath/ath9k/htc_drv_main.c | 2 +-
drivers/net/wireless/ath/ath9k/init.c | 4 ++--
8 files changed, 24 insertions(+), 18 deletions(-)
diff --git a/drivers/net/wireless/ath/ath9k/Kconfig
b/drivers/net/wireless/ath/ath9k/Kconfig
index a84bb9b6573f..6193dd4d85f0 100644
--- a/drivers/net/wireless/ath/ath9k/Kconfig
+++ b/drivers/net/wireless/ath/ath9k/Kconfig
@@ -23,9 +23,6 @@ config ATH9K
depends on MAC80211 && HAS_DMA
select ATH9K_HW
select ATH9K_COMMON
- imply NEW_LEDS
- imply LEDS_CLASS
- imply MAC80211_LEDS
help
This module adds support for wireless adapters based on
Atheros IEEE 802.11n AR5008, AR9001 and AR9002 family
@@ -38,6 +35,18 @@ config ATH9K
If you choose to build a module, it'll be called ath9k.
+config ATH9K_LEDS
+ bool "Atheros ath9k LED triggers"
+ default y
+ depends on ATH9K || ATH9K_HTC
+ depends on NEW_LEDS
+ select LEDS_CLASS
+ select MAC80211_LEDS
+ help
+ This option enables a few LED triggers for different
+ packet receive/transmit events on Atheros family
+ of wireless cards (PCI and HTC).
+
config ATH9K_PCI
bool "Atheros ath9k PCI/PCIe bus support"
default y
@@ -178,9 +187,6 @@ config ATH9K_HTC
depends on USB && MAC80211
select ATH9K_HW
select ATH9K_COMMON
- imply NEW_LEDS
- imply LEDS_CLASS
- imply MAC80211_LEDS
help
Support for Atheros HTC based cards.
Chipsets supported: AR9271
diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h
b/drivers/net/wireless/ath/ath9k/ath9k.h
index 13b4f5f50f8a..045118dc2a84 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -839,7 +839,7 @@ static inline int ath9k_dump_btcoex(struct ath_softc *sc,
u8 *buf, u32 size)
#define ATH_LED_PIN_9485 6
#define ATH_LED_PIN_9462 4
-#ifdef CONFIG_MAC80211_LEDS
+#ifdef CONFIG_ATH9K_LEDS
void ath_init_leds(struct ath_softc *sc);
void ath_deinit_leds(struct ath_softc *sc);
#else
@@ -1030,7 +1030,7 @@ struct ath_softc {
struct ath_chanctx *cur_chan;
spinlock_t chan_lock;
-#ifdef CONFIG_MAC80211_LEDS
+#ifdef CONFIG_ATH9K_LEDS
bool led_registered;
char led_name[32];
struct led_classdev led_cdev;
diff --git a/drivers/net/wireless/ath/ath9k/gpio.c
b/drivers/net/wireless/ath/ath9k/gpio.c
index b457e52dd365..aeaa7752049d 100644
--- a/drivers/net/wireless/ath/ath9k/gpio.c
+++ b/drivers/net/wireless/ath/ath9k/gpio.c
@@ -20,7 +20,7 @@
/* LED functions */
//
-#ifdef CONFIG_MAC80211_LEDS
+#ifdef CONFIG_ATH9K_LEDS
static void ath_fill_led_pin(struct ath_softc *sc)
{
diff --git a/drivers/net/wireless/ath/ath9k/htc.h
b/drivers/net/wireless/ath/ath9k/htc.h
index 0a1634238e67..d3a25c8bfcb5 100644
--- a/drivers/net/wireless/ath/ath9k/htc.h
+++ b/drivers/net/wireless/ath/ath9k/htc.h
@@ -44,7 +44,7 @@
extern struct ieee80211_ops ath9k_htc_ops;
extern int htc_modparam_nohwcrypt;
-#ifdef CONFIG_MAC80211_LEDS
+#ifdef CONFIG_ATH9K_LEDS
extern int ath9k_htc_led_blink;
#endif
@@ -510,7 +510,7 @@ struct ath9k_htc_priv {
bool ps_enabled;
bool ps_idle;
-#ifdef CONFIG_MAC80211_LEDS
+#ifdef CONFIG_ATH9K_LEDS
enum led_brightness brightness;
bool led_registered;
char led_name[32];
@@ -604,7 +604,7 @@ void ath9k_htc_rfkill_poll_state(struct ieee80211_hw *hw);
struct base_eep_header *ath9k_htc_get_eeprom_base(struct ath9k_htc_priv *priv);
-#ifdef CONFIG_MAC80211_LEDS
+#ifdef CONFIG_ATH9K_LEDS
void ath9k_configure_leds(struct ath9k_htc_priv *priv);
void ath9k_init_le
Re: [PATCH v4 4/7] power: supply: max8997_charger: Set CHARGER current limit
On Thu, Dec 24, 2020 at 02:37:06PM +0100, Krzysztof Kozlowski wrote: > On Thu, Dec 24, 2020 at 01:13:02PM +, Timon Baetz wrote: > > On Thu, 24 Dec 2020 10:55:59 +0100, Krzysztof Kozlowski wrote: > > > > @@ -170,6 +237,28 @@ static int max8997_battery_probe(struct > > > > platform_device *pdev) > > > > return PTR_ERR(charger->battery); > > > > } > > > > > > > > + charger->reg = devm_regulator_get(>dev, "charger"); > > > > > > Since you do not use get_optional, you will always get a dummy > > > regulator. In case of error, you should either print it or entirely fail > > > the probe. Silently continuing makes it difficult to spot errors. > > > > > > Since the driver could operate in case of extcon/regulator error, just > > > dev_err() so failure will be spotted with dmesg. > > > > I will switch to devm_regulator_get_optional() and print an error on > > failure, thanks. > > > > > It will complain on older DTBs because you are introducing incompatible > > > change, but that's expected. Just correct all other in-tree DTS. > > > > The other 2 in-tree DTS don't have CHARGER regulators. Not sure > > how to correct those. Should I add muic and charger nodes without a > > charger-supply? It will still complain in that case. > > +Cc Marek, > > This is why leaving the code as is - devm_regulator_get(), not optional > - makes sense. Core would provide dummy regulator, so you only have to > provide MUIC node. > > If you change the code to devm_regulator_get_optional(), you need to add > everything: the charger regulator, the charger node and MUIC node. > > For Trats, the configuration should be similar as i9100, although I > don't know the exact values of chargign voltage. > > For Origen, there is no battery, so the power supply should not bind. > Maybe this could be achieved with "status disabled" for charger node? It > depends whether MFD will respect such field... If it disables the > charger, you're done. I just looked at the MFD code and tested it - it nicely skips disabled devices. Therefore, for Origen I propose to add disabled nodes for charger and MUIC because these pins are not connected. No need to add regulators in such case. Best regards, Krzysztof
Re: [PATCH v4 4/7] power: supply: max8997_charger: Set CHARGER current limit
On Thu, Dec 24, 2020 at 01:13:02PM +, Timon Baetz wrote:
> On Thu, 24 Dec 2020 10:55:59 +0100, Krzysztof Kozlowski wrote:
> > On Wed, Dec 23, 2020 at 01:43:05PM +, Timon Baetz wrote:
> > > Register for extcon notification and set charging current depending on
> > > the detected cable type. Current values are taken from vendor kernel,
> > > where most charger types end up setting 650mA [0].
> > >
> > > Also enable and disable the CHARGER regulator based on extcon events.
> > >
> > > [0]
> > > https://github.com/krzk/linux-vendor-backup/blob/samsung/galaxy-s2-epic-4g-touch-sph-d710-exynos4210-dump/drivers/misc/max8997-muic.c#L1675-L1678
> > >
> > > Signed-off-by: Timon Baetz
> > > ---
> > > drivers/power/supply/max8997_charger.c | 89 ++
> > > 1 file changed, 89 insertions(+)
> > >
> > > diff --git a/drivers/power/supply/max8997_charger.c
> > > b/drivers/power/supply/max8997_charger.c
> > > index 1947af25879a..e8532e2af451 100644
> > > --- a/drivers/power/supply/max8997_charger.c
> > > +++ b/drivers/power/supply/max8997_charger.c
> > > @@ -6,12 +6,14 @@
> > > // MyungJoo Ham
> > >
> > > #include
> > > +#include
> > > #include
> > > #include
> > > #include
> > > #include
> > > #include
> > > #include
> > > +#include
> > >
> > > /* MAX8997_REG_STATUS4 */
> > > #define DCINOK_SHIFT 1
> > > @@ -31,6 +33,10 @@ struct charger_data {
> > > struct device *dev;
> > > struct max8997_dev *iodev;
> > > struct power_supply *battery;
> > > + struct regulator *reg;
> > > + struct extcon_dev *edev;
> > > + struct notifier_block extcon_nb;
> > > + struct work_struct extcon_work;
> > > };
> > >
> > > static enum power_supply_property max8997_battery_props[] = {
> > > @@ -88,6 +94,67 @@ static int max8997_battery_get_property(struct
> > > power_supply *psy,
> > > return 0;
> > > }
> > >
> > > +static void max8997_battery_extcon_evt_stop_work(void *data)
> > > +{
> > > + struct charger_data *charger = data;
> > > +
> > > + cancel_work_sync(>extcon_work);
> > > +}
> > > +
> > > +static void max8997_battery_extcon_evt_worker(struct work_struct *work)
> > > +{
> > > + struct charger_data *charger =
> > > + container_of(work, struct charger_data, extcon_work);
> > > + struct extcon_dev *edev = charger->edev;
> > > + int current_limit;
> > > +
> > > + if (extcon_get_state(edev, EXTCON_CHG_USB_SDP) > 0) {
> > > + dev_dbg(charger->dev, "USB SDP charger is connected\n");
> > > + current_limit = 45;
> > > + } else if (extcon_get_state(edev, EXTCON_CHG_USB_DCP) > 0) {
> > > + dev_dbg(charger->dev, "USB DCP charger is connected\n");
> > > + current_limit = 65;
> > > + } else if (extcon_get_state(edev, EXTCON_CHG_USB_FAST) > 0) {
> > > + dev_dbg(charger->dev, "USB FAST charger is connected\n");
> > > + current_limit = 65;
> > > + } else if (extcon_get_state(edev, EXTCON_CHG_USB_SLOW) > 0) {
> > > + dev_dbg(charger->dev, "USB SLOW charger is connected\n");
> > > + current_limit = 65;
> > > + } else if (extcon_get_state(edev, EXTCON_CHG_USB_CDP) > 0) {
> > > + dev_dbg(charger->dev, "USB CDP charger is connected\n");
> > > + current_limit = 65;
> > > + } else {
> > > + dev_dbg(charger->dev, "USB charger is diconnected\n");
> > > + current_limit = -1;
> > > + }
> > > +
> > > + if (current_limit > 0) {
> > > + int ret = regulator_set_current_limit(charger->reg,
> > > current_limit, current_limit);
> > > +
> > > + if (ret) {
> > > + dev_err(charger->dev, "failed to set current limit:
> > > %d\n", ret);
> > > + return;
> > > + }
> > > + ret = regulator_enable(charger->reg);
> > > + if (ret)
> > > + dev_err(charger->dev, "failed to enable regulator:
> > > %d\n", ret);
> > > + } else {
> > > + int ret = regulator_disable(charger
Re: [PATCH 2/2] arm64: dts: imx8mm: Add Gateworks IMX8MM Development Kits
On Wed, Dec 23, 2020 at 02:23:16PM -0800, Tim Harvey wrote:
> The Gateworks Venice GW71xx-0x/GW72xx-0x/GW73xx-0x are development
> kits comprised of a GW700x SoM and a Baseboard.
>
> The GW700x SoM contains:
> - IMX8MM SoC
> - LPDDR4 DRAM
> - eMMC FLASH
> - Gateworks System Controller (eeprom/pushbutton/reset/voltage-monitor)
> - GbE PHY connected to the IMX8MM FEC
> - Power Management IC
>
> The GW71xx Baseboard contains:
> - 1x MiniPCIe Socket with USB2.0, PCIe, and SIM
> - 1x RJ45 GbE (IMX8MM FEC)
> - PCIe Clock generator
> - GPS and accelerometer
> - 1x USB 2.0 Front Panel connector
> - wide range power supply
>
> The GW72xx Baseboard contains:
> - 2x MiniPCIe Socket with USB2.0, PCIe, and SIM
> - 2x RJ45 GbE (IMX8MM FEC and LAN743x)
> - 1x MicroSD connector
> - 1x USB 2.0 Front Panel connector
> - 1x SPI connector
> - PCIe Clock generator
> - GPS and accelerometer
> - Media Expansion connector (MIPI-CSI/MIPI-DSI/GPIO/I2S)
> - wide range power supply
>
> The GW73xx Baseboard contains:
> - 3x MiniPCIe Socket with USB2.0, PCIe, and SIM
> - 2x RJ45 GbE (IMX8MM FEC and LAN743x)
> - 1x MicroSD connector
> - 1x USB 2.0 Front Panel connector
> - 1x SPI connector
> - WiFi/BT
> - PCIe Clock generator
> - GPS and accelerometer
> - Media Expansion connector (MIPI-CSI/MIPI-DSI/GPIO/I2S)
> - wide range power supply
>
> Signed-off-by: Tim Harvey
> ---
> arch/arm64/boot/dts/freescale/Makefile | 3 +
> .../boot/dts/freescale/imx8mm-venice-gw700x.dtsi | 482
> +
> .../boot/dts/freescale/imx8mm-venice-gw71xx-0x.dts | 19 +
> .../boot/dts/freescale/imx8mm-venice-gw71xx.dtsi | 186
> .../boot/dts/freescale/imx8mm-venice-gw72xx-0x.dts | 20 +
> .../boot/dts/freescale/imx8mm-venice-gw72xx.dtsi | 311 +
> .../boot/dts/freescale/imx8mm-venice-gw73xx-0x.dts | 19 +
> .../boot/dts/freescale/imx8mm-venice-gw73xx.dtsi | 363
> 8 files changed, 1403 insertions(+)
> create mode 100644 arch/arm64/boot/dts/freescale/imx8mm-venice-gw700x.dtsi
> create mode 100644 arch/arm64/boot/dts/freescale/imx8mm-venice-gw71xx-0x.dts
> create mode 100644 arch/arm64/boot/dts/freescale/imx8mm-venice-gw71xx.dtsi
> create mode 100644 arch/arm64/boot/dts/freescale/imx8mm-venice-gw72xx-0x.dts
> create mode 100644 arch/arm64/boot/dts/freescale/imx8mm-venice-gw72xx.dtsi
> create mode 100644 arch/arm64/boot/dts/freescale/imx8mm-venice-gw73xx-0x.dts
> create mode 100644 arch/arm64/boot/dts/freescale/imx8mm-venice-gw73xx.dtsi
>
> diff --git a/arch/arm64/boot/dts/freescale/Makefile
> b/arch/arm64/boot/dts/freescale/Makefile
> index f8d5943..ecdd233 100644
> --- a/arch/arm64/boot/dts/freescale/Makefile
> +++ b/arch/arm64/boot/dts/freescale/Makefile
> @@ -32,6 +32,9 @@ dtb-$(CONFIG_ARCH_MXC) += imx8mm-beacon-kit.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mm-evk.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mm-ddr4-evk.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mm-var-som-symphony.dtb
> +dtb-$(CONFIG_ARCH_MXC) += imx8mm-venice-gw71xx-0x.dtb
> +dtb-$(CONFIG_ARCH_MXC) += imx8mm-venice-gw72xx-0x.dtb
> +dtb-$(CONFIG_ARCH_MXC) += imx8mm-venice-gw73xx-0x.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mn-evk.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mn-ddr4-evk.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mn-var-som-symphony.dtb
(...)
I have few more thoughts below.
> diff --git a/arch/arm64/boot/dts/freescale/imx8mm-venice-gw71xx.dtsi
> b/arch/arm64/boot/dts/freescale/imx8mm-venice-gw71xx.dtsi
> new file mode 100644
> index ..a4eeb0d
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx8mm-venice-gw71xx.dtsi
> @@ -0,0 +1,186 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright 2020 Gateworks Corporation
> + */
> +
> +/ {
> + aliases {
> + usb0 =
> + usb1 =
> + };
> +
> + leds {
> + compatible = "gpio-leds";
> + pinctrl-names = "default";
> + pinctrl-0 = <_gpio_leds>;
> +
> + user1 { /* GRN */
> + label = "user1";
> + gpios = < 5 GPIO_ACTIVE_HIGH>;
> + default-state = "on";
> + linux,default-trigger = "heartbeat";
> + };
> +
> + user2 { /* RED */
> + label = "user2";
> + gpios = < 4 GPIO_ACTIVE_HIGH>;
> + default-state = "off";
> + };
> + };
> +
> + pps {
> + compatible = "pps-gpio";
> + pinctrl-names = "default";
> + pinctrl-0 = <_pps>;
> + gpios = < 15 GPIO_ACTIVE_HIGH>;
> + status = "okay";
> + };
> +
> + reg_usb_otg1_vbus: regulator-usb-otg1 {
> + pinctrl-names = "default";
> + pinctrl-0 = <_reg_usb1_en>;
> + compatible = "regulator-fixed";
> + regulator-name = "usb_otg1_vbus";
> + gpio = < 12 GPIO_ACTIVE_HIGH>;
> + enable-active-high;
> +
Re: [PATCH 2/2] arm64: dts: imx8mm: Add Gateworks IMX8MM Development Kits
On Wed, Dec 23, 2020 at 02:23:16PM -0800, Tim Harvey wrote:
> The Gateworks Venice GW71xx-0x/GW72xx-0x/GW73xx-0x are development
> kits comprised of a GW700x SoM and a Baseboard.
>
> The GW700x SoM contains:
> - IMX8MM SoC
It's i.MX 8M Mini.
https://www.nxp.com/products/processors-and-microcontrollers/arm-processors/i-mx-applications-processors/i-mx-8-processors/i-mx-8m-mini-arm-cortex-a53-cortex-m4-audio-voice-video:i.MX8MMINI
> - LPDDR4 DRAM
> - eMMC FLASH
> - Gateworks System Controller (eeprom/pushbutton/reset/voltage-monitor)
> - GbE PHY connected to the IMX8MM FEC
> - Power Management IC
>
> The GW71xx Baseboard contains:
> - 1x MiniPCIe Socket with USB2.0, PCIe, and SIM
> - 1x RJ45 GbE (IMX8MM FEC)
> - PCIe Clock generator
> - GPS and accelerometer
> - 1x USB 2.0 Front Panel connector
> - wide range power supply
>
> The GW72xx Baseboard contains:
> - 2x MiniPCIe Socket with USB2.0, PCIe, and SIM
> - 2x RJ45 GbE (IMX8MM FEC and LAN743x)
> - 1x MicroSD connector
> - 1x USB 2.0 Front Panel connector
> - 1x SPI connector
> - PCIe Clock generator
> - GPS and accelerometer
> - Media Expansion connector (MIPI-CSI/MIPI-DSI/GPIO/I2S)
> - wide range power supply
>
> The GW73xx Baseboard contains:
> - 3x MiniPCIe Socket with USB2.0, PCIe, and SIM
> - 2x RJ45 GbE (IMX8MM FEC and LAN743x)
> - 1x MicroSD connector
> - 1x USB 2.0 Front Panel connector
> - 1x SPI connector
> - WiFi/BT
> - PCIe Clock generator
> - GPS and accelerometer
> - Media Expansion connector (MIPI-CSI/MIPI-DSI/GPIO/I2S)
> - wide range power supply
>
> Signed-off-by: Tim Harvey
> ---
> arch/arm64/boot/dts/freescale/Makefile | 3 +
> .../boot/dts/freescale/imx8mm-venice-gw700x.dtsi | 482
> +
> .../boot/dts/freescale/imx8mm-venice-gw71xx-0x.dts | 19 +
> .../boot/dts/freescale/imx8mm-venice-gw71xx.dtsi | 186
> .../boot/dts/freescale/imx8mm-venice-gw72xx-0x.dts | 20 +
> .../boot/dts/freescale/imx8mm-venice-gw72xx.dtsi | 311 +
> .../boot/dts/freescale/imx8mm-venice-gw73xx-0x.dts | 19 +
> .../boot/dts/freescale/imx8mm-venice-gw73xx.dtsi | 363
> 8 files changed, 1403 insertions(+)
> create mode 100644 arch/arm64/boot/dts/freescale/imx8mm-venice-gw700x.dtsi
> create mode 100644 arch/arm64/boot/dts/freescale/imx8mm-venice-gw71xx-0x.dts
> create mode 100644 arch/arm64/boot/dts/freescale/imx8mm-venice-gw71xx.dtsi
> create mode 100644 arch/arm64/boot/dts/freescale/imx8mm-venice-gw72xx-0x.dts
> create mode 100644 arch/arm64/boot/dts/freescale/imx8mm-venice-gw72xx.dtsi
> create mode 100644 arch/arm64/boot/dts/freescale/imx8mm-venice-gw73xx-0x.dts
> create mode 100644 arch/arm64/boot/dts/freescale/imx8mm-venice-gw73xx.dtsi
>
> diff --git a/arch/arm64/boot/dts/freescale/Makefile
> b/arch/arm64/boot/dts/freescale/Makefile
> index f8d5943..ecdd233 100644
> --- a/arch/arm64/boot/dts/freescale/Makefile
> +++ b/arch/arm64/boot/dts/freescale/Makefile
> @@ -32,6 +32,9 @@ dtb-$(CONFIG_ARCH_MXC) += imx8mm-beacon-kit.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mm-evk.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mm-ddr4-evk.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mm-var-som-symphony.dtb
> +dtb-$(CONFIG_ARCH_MXC) += imx8mm-venice-gw71xx-0x.dtb
> +dtb-$(CONFIG_ARCH_MXC) += imx8mm-venice-gw72xx-0x.dtb
> +dtb-$(CONFIG_ARCH_MXC) += imx8mm-venice-gw73xx-0x.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mn-evk.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mn-ddr4-evk.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mn-var-som-symphony.dtb
> diff --git a/arch/arm64/boot/dts/freescale/imx8mm-venice-gw700x.dtsi
> b/arch/arm64/boot/dts/freescale/imx8mm-venice-gw700x.dtsi
> new file mode 100644
> index ..7e1b96b
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx8mm-venice-gw700x.dtsi
> @@ -0,0 +1,482 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright 2020 Gateworks Corporation
> + */
> +
> +#include
> +#include
> +#include
> +
> +/ {
> + memory@4000 {
> + device_type = "memory";
> + reg = <0x0 0x4000 0 0x8000>;
> + };
> +
> + gpio_keys {
> + compatible = "gpio-keys";
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + user_pb {
> + label = "user_pb";
> + gpios = <_gpio 2 GPIO_ACTIVE_LOW>;
> + linux,code = ;
> + };
> +
> + user_pb1x {
> + label = "user_pb1x";
> + linux,code = ;
> + interrupt-parent = <>;
> + interrupts = <0>;
> + };
> +
> + key_erased {
> + label = "key-erased";
Above you use underscore, here hyphen. Make it consistent.
> + linux,code = ;
> + interrupt-parent = <>;
> + interrupts = <1>;
> + };
> +
> + eeprom_wp {
> + label =
Re: [PATCH 1/2] dt-bindings: arm: fsl: Add binding for Gateworks boards with IMX8MM
On Wed, Dec 23, 2020 at 02:23:15PM -0800, Tim Harvey wrote: > Add bindings for the Gateworks Venice Development kit boards with > IMX8MM System on Module. > > Signed-off-by: Tim Harvey > --- > Documentation/devicetree/bindings/arm/fsl.yaml | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml > b/Documentation/devicetree/bindings/arm/fsl.yaml > index 1ca9dfa..705c6e8 100644 > --- a/Documentation/devicetree/bindings/arm/fsl.yaml > +++ b/Documentation/devicetree/bindings/arm/fsl.yaml > @@ -405,6 +405,9 @@ properties: >- beacon,imx8mm-beacon-kit # i.MX8MM Beacon Development Kit >- fsl,imx8mm-ddr4-evk # i.MX8MM DDR4 EVK Board >- fsl,imx8mm-evk# i.MX8MM EVK Board > + - gw,imx8mm-gw71xx-0x # i.MX8MM Gateworks Development Kit > + - gw,imx8mm-gw72xx-0x # i.MX8MM Gateworks Development Kit > + - gw,imx8mm-gw73xx-0x # i.MX8MM Gateworks Development Kit I think you should skip the wildcards in compatible and choose one specific compatible. What if at some point you would like to add gw,imx8mm-gw7113-0x? Best regards, Krzysztof >- variscite,var-som-mx8mm # i.MX8MM Variscite VAR-SOM-MX8MM > module >- const: fsl,imx8mm > > -- > 2.7.4 >
Re: [PATCH v3 4/6] arm64: dts: imx8mm: Add Engicam i.Core MX8M Mini C.TOUCH 2.0
On Thu, 24 Dec 2020 at 11:08, Jagan Teki wrote: > > On Thu, Dec 24, 2020 at 2:48 PM Krzysztof Kozlowski wrote: > > > > On Wed, 23 Dec 2020 at 13:07, Jagan Teki wrote: > > > > > > On Wed, Dec 23, 2020 at 5:29 PM Krzysztof Kozlowski > > > wrote: > > > > > > > > On Wed, Dec 23, 2020 at 04:33:41PM +0530, Jagan Teki wrote: > > > > > Engicam C.TOUCH 2.0 is an EDIMM compliant general purpose Carrier > > > > > board. > > > > > > > > > > Genaral features: > > > > > - Ethernet 10/100 > > > > > - Wifi/BT > > > > > - USB Type A/OTG > > > > > - Audio Out > > > > > - CAN > > > > > - LVDS panel connector > > > > > > > > > > i.Core MX8M Mini is an EDIMM SoM based on NXP i.MX8M Mini from > > > > > Engicam. > > > > > > > > > > i.Core MX8M Mini needs to mount on top of this Carrier board for > > > > > creating complete i.Core MX8M Mini C.TOUCH 2.0 board. > > > > > > > > > > Add support for it. > > > > > > > > > > Signed-off-by: Matteo Lisi > > > > > Signed-off-by: Jagan Teki > > > > > --- > > > > > Changes for v3: > > > > > - don't maintain common nodes and include it, if no feature diff > > > > > Changes for v2: > > > > > - enabled fec1 node > > > > > - updated commit message > > > > > - dropped engicam from filename since it aligned with imx6 engicam > > > > > dts files naming conventions. > > > > > - add i2c nodes > > > > > - fixed v1 comments > > > > > > > > > > arch/arm64/boot/dts/freescale/Makefile| 1 + > > > > > .../dts/freescale/imx8mm-engicam-ctouch2.dtsi | 82 > > > > > +++ > > > > > .../freescale/imx8mm-icore-mx8mm-ctouch2.dts | 21 + > > > > > 3 files changed, 104 insertions(+) > > > > > create mode 100644 > > > > > arch/arm64/boot/dts/freescale/imx8mm-engicam-ctouch2.dtsi > > > > > > > > You split some common part to ctouch2.dtsi so it can be reused in > > > > multiple places. I saw so far only one usage, where are the others? > > > > > > To be clear, ctouch2.dtsi not mean for common it is C.TOUCH2 carrier > > > board dtsi. The other carrier is C.TOUCH2 10.1" Open Frame(display), > > > since DSI is not yet mainlined, I didn't add this yet. > > > > If I understand correctly: it is a DTSI which is included only by one > > DTS... and DTS does not have any other nodes. This as well is not the > > This is not mandatory as per my understanding, including exiting DTS > topologies in Mainline. > > There are several places where more than one dtsi has been included, > Simple example of imx8mm tree is It's not the problem of including more than one DTSI. It's the problem of creating fake DTS or DTSI files whose purpose is only to include others. Keep it simple. Don't create unnecessary files. "Entities should not be multiplied without necessity." > > arch/arm64/boot/dts/freescale/imx8mm-beacon-kit.dts Which was wrong as well. Don't create unnecessary files. > > /dts-v1/; > > #include "imx8mm.dtsi" > #include "imx8mm-beacon-som.dtsi" > #include "imx8mm-beacon-baseboard.dtsi" > > (SoC dtsi, SoM dtsi, Carrier board dtsi) > > > design which makes any sense. We do not create empty DTS files which > > only include one more DTSI. The contents of > > imx8mm-engicam-ctouch2.dtsi should be directly in > > imx8mm-icore-mx8mm-ctouch2.dts. That's the same problem as with v1 - > > you overcomplicate simple stuff. It really looks like you ignored the > > comments from v1 in multiple places. > > As explained above, the design is pretty much the same as the existing SoM's. > > imx8mm-engicam-ctouch2.dtsi is not just a dtsi file where nodes are > enabled. It has nodes enabled for Carrier board, so keeping nodes > separately will The files represent real devices or their components. So you have a SOM - a DTSI file. You have a carrier board - a DTS file. That's simple design which is mostly followed, unless something over complicated passes the review. > 1. More verbose for which IP's are available in the carrier board No difference when carrier DTSI is the DTS. Exactly the same. > 2. Easy to extend if someone can create another SoM with a similar Carrier. Not really, if they include carrier DTSI they need to override a lot. So usually (including practice - I did it) they *copy* the carrier to create their own design. > > Ie is the whole idea to keep carrier board dtsi and includes them in dts. > > As I suggest, if you can look into px30 you can understand more easily. NAK from my side. I explained my reasoning. You created a fake, empty DTSI which included only other DTSI. After review, you agreed to fix it. However you still create a fake DTS which includes only a DTSI. "Entities should not be multiplied without necessity." Best regards, Krzysztof
Re: [PATCH v4 4/7] power: supply: max8997_charger: Set CHARGER current limit
On Wed, Dec 23, 2020 at 01:43:05PM +, Timon Baetz wrote:
> Register for extcon notification and set charging current depending on
> the detected cable type. Current values are taken from vendor kernel,
> where most charger types end up setting 650mA [0].
>
> Also enable and disable the CHARGER regulator based on extcon events.
>
> [0]
> https://github.com/krzk/linux-vendor-backup/blob/samsung/galaxy-s2-epic-4g-touch-sph-d710-exynos4210-dump/drivers/misc/max8997-muic.c#L1675-L1678
>
> Signed-off-by: Timon Baetz
> ---
> drivers/power/supply/max8997_charger.c | 89 ++
> 1 file changed, 89 insertions(+)
>
> diff --git a/drivers/power/supply/max8997_charger.c
> b/drivers/power/supply/max8997_charger.c
> index 1947af25879a..e8532e2af451 100644
> --- a/drivers/power/supply/max8997_charger.c
> +++ b/drivers/power/supply/max8997_charger.c
> @@ -6,12 +6,14 @@
> // MyungJoo Ham
>
> #include
> +#include
> #include
> #include
> #include
> #include
> #include
> #include
> +#include
>
> /* MAX8997_REG_STATUS4 */
> #define DCINOK_SHIFT 1
> @@ -31,6 +33,10 @@ struct charger_data {
> struct device *dev;
> struct max8997_dev *iodev;
> struct power_supply *battery;
> + struct regulator *reg;
> + struct extcon_dev *edev;
> + struct notifier_block extcon_nb;
> + struct work_struct extcon_work;
> };
>
> static enum power_supply_property max8997_battery_props[] = {
> @@ -88,6 +94,67 @@ static int max8997_battery_get_property(struct
> power_supply *psy,
> return 0;
> }
>
> +static void max8997_battery_extcon_evt_stop_work(void *data)
> +{
> + struct charger_data *charger = data;
> +
> + cancel_work_sync(>extcon_work);
> +}
> +
> +static void max8997_battery_extcon_evt_worker(struct work_struct *work)
> +{
> + struct charger_data *charger =
> + container_of(work, struct charger_data, extcon_work);
> + struct extcon_dev *edev = charger->edev;
> + int current_limit;
> +
> + if (extcon_get_state(edev, EXTCON_CHG_USB_SDP) > 0) {
> + dev_dbg(charger->dev, "USB SDP charger is connected\n");
> + current_limit = 45;
> + } else if (extcon_get_state(edev, EXTCON_CHG_USB_DCP) > 0) {
> + dev_dbg(charger->dev, "USB DCP charger is connected\n");
> + current_limit = 65;
> + } else if (extcon_get_state(edev, EXTCON_CHG_USB_FAST) > 0) {
> + dev_dbg(charger->dev, "USB FAST charger is connected\n");
> + current_limit = 65;
> + } else if (extcon_get_state(edev, EXTCON_CHG_USB_SLOW) > 0) {
> + dev_dbg(charger->dev, "USB SLOW charger is connected\n");
> + current_limit = 65;
> + } else if (extcon_get_state(edev, EXTCON_CHG_USB_CDP) > 0) {
> + dev_dbg(charger->dev, "USB CDP charger is connected\n");
> + current_limit = 65;
> + } else {
> + dev_dbg(charger->dev, "USB charger is diconnected\n");
> + current_limit = -1;
> + }
> +
> + if (current_limit > 0) {
> + int ret = regulator_set_current_limit(charger->reg,
> current_limit, current_limit);
> +
> + if (ret) {
> + dev_err(charger->dev, "failed to set current limit:
> %d\n", ret);
> + return;
> + }
> + ret = regulator_enable(charger->reg);
> + if (ret)
> + dev_err(charger->dev, "failed to enable regulator:
> %d\n", ret);
> + } else {
> + int ret = regulator_disable(charger->reg);
> +
> + if (ret)
> + dev_err(charger->dev, "failed to disable regulator:
> %d\n", ret);
> + }
> +}
> +
> +static int max8997_battery_extcon_evt(struct notifier_block *nb,
> + unsigned long event, void *param)
> +{
> + struct charger_data *charger =
> + container_of(nb, struct charger_data, extcon_nb);
> + schedule_work(>extcon_work);
> + return NOTIFY_OK;
> +}
> +
> static const struct power_supply_desc max8997_battery_desc = {
> .name = "max8997_pmic",
> .type = POWER_SUPPLY_TYPE_BATTERY,
> @@ -170,6 +237,28 @@ static int max8997_battery_probe(struct platform_device
> *pdev)
> return PTR_ERR(charger->battery);
> }
>
> + charger->reg = devm_regulator_get(>dev, "charger");
Since you do not use get_optional, you will always get a dummy
regulator. In case of error, you should either print it or entirely fail
the probe. Silently continuing makes it difficult to spot errors.
Since the driver could operate in case of extcon/regulator error, just
dev_err() so failure will be spotted with dmesg.
It will complain on older DTBs because you are introducing incompatible
change, but that's expected. Just correct all other in-tree DTS.
Best regards,
Krzysztof
> + charger->edev = extcon_get_edev_by_phandle(>dev,
Re: [PATCH v4 2/7] regulator: dt-bindings: Document max8997-pmic nodes
On Wed, Dec 23, 2020 at 01:42:43PM +, Timon Baetz wrote: > Add maxim,max8997-battery and maxim,max8997-muic optional nodes. > > Signed-off-by: Timon Baetz I already acked this, why did you skip my tag? Best regards, Krzysztof > --- > .../bindings/regulator/max8997-regulator.txt | 12 > 1 file changed, 12 insertions(+) > > diff --git > a/Documentation/devicetree/bindings/regulator/max8997-regulator.txt > b/Documentation/devicetree/bindings/regulator/max8997-regulator.txt > index 6fe825b8ac1b..faaf2bbf0272 100644 > --- a/Documentation/devicetree/bindings/regulator/max8997-regulator.txt > +++ b/Documentation/devicetree/bindings/regulator/max8997-regulator.txt > @@ -53,6 +53,18 @@ Additional properties required if either of the optional > properties are used: > - max8997,pmic-buck125-dvs-gpios: GPIO specifiers for three host gpio's used >for dvs. The format of the gpio specifier depends in the gpio controller. > > +Optional nodes: > +- charger: Node for configuring the charger driver. > + Required properties: > + - compatible: "maxim,max8997-battery" > + Optional properties: > + - extcon: extcon specifier for charging events > + - charger-supply: regulator node for charging current > + > +- muic: Node used only by extcon consumers. > + Required properties: > + - compatible: "maxim,max8997-muic" > + > Regulators: The regulators of max8997 that have to be instantiated should be > included in a sub-node named 'regulators'. Regulator nodes included in this > sub-node should be of the format as listed below. > -- > 2.25.1 > >
Re: [PATCH v3 4/6] arm64: dts: imx8mm: Add Engicam i.Core MX8M Mini C.TOUCH 2.0
On Wed, 23 Dec 2020 at 13:07, Jagan Teki wrote: > > On Wed, Dec 23, 2020 at 5:29 PM Krzysztof Kozlowski wrote: > > > > On Wed, Dec 23, 2020 at 04:33:41PM +0530, Jagan Teki wrote: > > > Engicam C.TOUCH 2.0 is an EDIMM compliant general purpose Carrier > > > board. > > > > > > Genaral features: > > > - Ethernet 10/100 > > > - Wifi/BT > > > - USB Type A/OTG > > > - Audio Out > > > - CAN > > > - LVDS panel connector > > > > > > i.Core MX8M Mini is an EDIMM SoM based on NXP i.MX8M Mini from Engicam. > > > > > > i.Core MX8M Mini needs to mount on top of this Carrier board for > > > creating complete i.Core MX8M Mini C.TOUCH 2.0 board. > > > > > > Add support for it. > > > > > > Signed-off-by: Matteo Lisi > > > Signed-off-by: Jagan Teki > > > --- > > > Changes for v3: > > > - don't maintain common nodes and include it, if no feature diff > > > Changes for v2: > > > - enabled fec1 node > > > - updated commit message > > > - dropped engicam from filename since it aligned with imx6 engicam > > > dts files naming conventions. > > > - add i2c nodes > > > - fixed v1 comments > > > > > > arch/arm64/boot/dts/freescale/Makefile| 1 + > > > .../dts/freescale/imx8mm-engicam-ctouch2.dtsi | 82 +++ > > > .../freescale/imx8mm-icore-mx8mm-ctouch2.dts | 21 + > > > 3 files changed, 104 insertions(+) > > > create mode 100644 > > > arch/arm64/boot/dts/freescale/imx8mm-engicam-ctouch2.dtsi > > > > You split some common part to ctouch2.dtsi so it can be reused in > > multiple places. I saw so far only one usage, where are the others? > > To be clear, ctouch2.dtsi not mean for common it is C.TOUCH2 carrier > board dtsi. The other carrier is C.TOUCH2 10.1" Open Frame(display), > since DSI is not yet mainlined, I didn't add this yet. If I understand correctly: it is a DTSI which is included only by one DTS... and DTS does not have any other nodes. This as well is not the design which makes any sense. We do not create empty DTS files which only include one more DTSI. The contents of imx8mm-engicam-ctouch2.dtsi should be directly in imx8mm-icore-mx8mm-ctouch2.dts. That's the same problem as with v1 - you overcomplicate simple stuff. It really looks like you ignored the comments from v1 in multiple places. The same applies to imx8mm-engicam-edimm2.2.dtsi. Best regards, Krzysztof
Re: [PATCH v3 4/6] arm64: dts: imx8mm: Add Engicam i.Core MX8M Mini C.TOUCH 2.0
On Wed, Dec 23, 2020 at 04:33:41PM +0530, Jagan Teki wrote: > Engicam C.TOUCH 2.0 is an EDIMM compliant general purpose Carrier > board. > > Genaral features: > - Ethernet 10/100 > - Wifi/BT > - USB Type A/OTG > - Audio Out > - CAN > - LVDS panel connector > > i.Core MX8M Mini is an EDIMM SoM based on NXP i.MX8M Mini from Engicam. > > i.Core MX8M Mini needs to mount on top of this Carrier board for > creating complete i.Core MX8M Mini C.TOUCH 2.0 board. > > Add support for it. > > Signed-off-by: Matteo Lisi > Signed-off-by: Jagan Teki > --- > Changes for v3: > - don't maintain common nodes and include it, if no feature diff > Changes for v2: > - enabled fec1 node > - updated commit message > - dropped engicam from filename since it aligned with imx6 engicam > dts files naming conventions. > - add i2c nodes > - fixed v1 comments > > arch/arm64/boot/dts/freescale/Makefile| 1 + > .../dts/freescale/imx8mm-engicam-ctouch2.dtsi | 82 +++ > .../freescale/imx8mm-icore-mx8mm-ctouch2.dts | 21 + > 3 files changed, 104 insertions(+) > create mode 100644 arch/arm64/boot/dts/freescale/imx8mm-engicam-ctouch2.dtsi You split some common part to ctouch2.dtsi so it can be reused in multiple places. I saw so far only one usage, where are the others? Best regards, Krzysztof
Re: [PATCH v3 3/6] arm64: dts: imx8mm: Add Engicam i.Core MX8M Mini SoM
On Wed, Dec 23, 2020 at 04:33:40PM +0530, Jagan Teki wrote: > i.Core MX8M Mini is an EDIMM SoM based on NXP i.MX8M Mini > from Engicam. > > General features: > - NXP i.MX8M Mini > - Up to 2GB LDDR4 > - 8/16GB eMMC > - Gigabit Ethernet > - USB 2.0 Host/OTG > - PCIe Gen2 interface > - I2S > - MIPI DSI to LVDS > - rest of i.MX8M Mini features > > i.Core MX8M Mini needs to mount on top of Engicam baseboards > for creating complete platform solutions. > > Add support for it. > > Signed-off-by: Matteo Lisi > Signed-off-by: Jagan Teki > --- > Changes for v3: > - keep regulator min/max hoping > Changes for v2: > - updated commit message > - add cpu nodes > - add fec1 node > - fixed pmic tree comments > - dropped engicam from filename since it aligned with imx6 engicam > dts files naming conventions. > > .../dts/freescale/imx8mm-icore-mx8mm.dtsi | 232 ++ > 1 file changed, 232 insertions(+) > create mode 100644 arch/arm64/boot/dts/freescale/imx8mm-icore-mx8mm.dtsi Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH v3 2/6] dt-bindings: arm: fsl: Add Engicam i.Core MX8M Mini C.TOUCH 2.0
On Wed, Dec 23, 2020 at 04:33:39PM +0530, Jagan Teki wrote: > i.Core MX8M Mini is an EDIMM SoM based on NXP i.MX8M Mini from Engicam. > > C.TOUCH 2.0 is a general purpose carrier board with capacitive > touch interface support. > > i.Core MX8M Mini needs to mount on top of this Carrier board for > creating complete i.Core MX8M Mini C.TOUCH 2.0 board. > > Add bindings for it. > > Signed-off-by: Jagan Teki > --- > Changes for v3: > - add proper bindings > Changes for v2: > - updated commit message > Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH v2 3/4] arm64: dts: imx8mq-librem5: Move usdhc clocks assignment to board DT
On Tue, Dec 22, 2020 at 04:13:46PM +0100, Martin Kepplinger wrote:
> According to commit e045f044e84e ("arm64: dts: imx8mq: Move usdhc clocks
> assignment to board DT") add the clocks assignment to imx8mq-librem5.dtsi
> too.
>
> Fixes: e045f044e84e ("arm64: dts: imx8mq: Move usdhc clocks assignment to
> board DT")
> Signed-off-by: Martin Kepplinger
> ---
> arch/arm64/boot/dts/freescale/imx8mq-librem5.dtsi | 4
> 1 file changed, 4 insertions(+)
Reviewed-by: Krzysztof Kozlowski
Best regards,
Krzysztof
Re: [PATCH v2 2/4] arm64: dts: imx8mq-librem5: add pinctrl for the touchscreen description
On Tue, Dec 22, 2020 at 04:13:45PM +0100, Martin Kepplinger wrote:
> In order for the touchscreen interrupt line to work, describe it properly.
> Otherwise it can work if defaults are ok, but we cannot be sure.
>
> Fixes: 8f0216b006e5 ("arm64: dts: Add a device tree for the Librem 5 phone")
> Signed-off-by: Martin Kepplinger
> ---
> arch/arm64/boot/dts/freescale/imx8mq-librem5.dtsi | 9 +
> 1 file changed, 9 insertions(+)
>
Reviewed-by: Krzysztof Kozlowski
Best regards,
Krzysztof
Re: [PATCH v3 4/7] power: supply: max8997_charger: Set CHARGER current limit
On Wed, Dec 23, 2020 at 08:09:55AM +, Timon Baetz wrote: > On Tue, 22 Dec 2020 09:40:04 +0100, Krzysztof Kozlowski wrote: (...) > > > .name = "max8997_pmic", > > > .type = POWER_SUPPLY_TYPE_BATTERY, > > > @@ -170,6 +237,33 @@ static int max8997_battery_probe(struct > > > platform_device *pdev) > > > return PTR_ERR(charger->battery); > > > } > > > > > > + charger->reg = devm_regulator_get(>dev, "charger"); > > > > The code looks good but isn't it breaking all existing platforms? > > So there is 2 other DTS in the kernel sources that are using MAX8997 > pmic: > - Insignal Origen evaluation board > - Samsung Trats > Non of them have charging regulators. But still the power supply was probing on them (if not the error you mentioned). Now, the charger will fail. > Also probing of the charger has been failing for long time because of > https://lore.kernel.org/lkml/20201109194251.562203-2-timon.ba...@protonmail.com/ > but that seems to land in 5.11. That's a good argument supporting introduced breakage. Use it in commit message. Don't hide such information. > That being said, I guess I could make extcon and charger-supply > optional if you prefer. Since at least two boards will loose now power supply, I don't think you have a choice. Best regards, Krzysztof
Re: [PATCH v2 2/6] dt-bindings: arm: fsl: Add Engicam i.Core MX8M Mini C.TOUCH 2.0
On Tue, Dec 22, 2020 at 09:25:40PM +0100, Krzysztof Kozlowski wrote: > On Tue, 22 Dec 2020 at 19:28, Jagan Teki wrote: > > > > On Mon, Dec 21, 2020 at 8:17 PM Jagan Teki > > wrote: > > > > > > On Mon, Dec 21, 2020 at 8:12 PM Krzysztof Kozlowski > > > wrote: > > > > > > > > On Mon, Dec 21, 2020 at 08:09:47PM +0530, Jagan Teki wrote: > > > > > On Mon, Dec 21, 2020 at 7:35 PM Krzysztof Kozlowski > > > > > wrote: > > > > > > > > > > > > On Mon, Dec 21, 2020 at 07:29:22PM +0530, Jagan Teki wrote: > > > > > > > On Mon, Dec 21, 2020 at 7:16 PM Krzysztof Kozlowski > > > > > > > wrote: > > > > > > > > > > > > > > > > On Mon, Dec 21, 2020 at 05:01:47PM +0530, Jagan Teki wrote: > > > > > > > > > i.Core MX8M Mini is an EDIMM SoM based on NXP i.MX8M Mini > > > > > > > > > from Engicam. > > > > > > > > > > > > > > > > > > C.TOUCH 2.0 is a general purpose carrier board with capacitive > > > > > > > > > touch interface support. > > > > > > > > > > > > > > > > > > i.Core MX8M Mini needs to mount on top of this Carrier board > > > > > > > > > for > > > > > > > > > creating complete i.Core MX8M Mini C.TOUCH 2.0 board. > > > > > > > > > > > > > > > > > > Add bindings for it. > > > > > > > > > > > > > > > > > > Signed-off-by: Jagan Teki > > > > > > > > > --- > > > > > > > > > Changes for v2: > > > > > > > > > - updated commit message > > > > > > > > > > > > > > > > > > Documentation/devicetree/bindings/arm/fsl.yaml | 2 ++ > > > > > > > > > 1 file changed, 2 insertions(+) > > > > > > > > > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml > > > > > > > > > b/Documentation/devicetree/bindings/arm/fsl.yaml > > > > > > > > > index 67980dcef66d..e653e0a43016 100644 > > > > > > > > > --- a/Documentation/devicetree/bindings/arm/fsl.yaml > > > > > > > > > +++ b/Documentation/devicetree/bindings/arm/fsl.yaml > > > > > > > > > @@ -667,6 +667,8 @@ properties: > > > > > > > > > items: > > > > > > > > >- enum: > > > > > > > > >- beacon,imx8mm-beacon-kit # i.MX8MM Beacon > > > > > > > > > Development Kit > > > > > > > > > + - engicam,icore-mx8mm # i.MX8MM > > > > > > > > > Engicam i.Core MX8M Mini SOM > > > > > > > > > + - engicam,icore-mx8mm-ctouch2 # i.MX8MM > > > > > > > > > Engicam i.Core MX8M Mini C.TOUCH 2.0 > > > > > > > > > > > > > > > > Please test your DTS against new schema with dtbs_check. This > > > > > > > > won't > > > > > > > > match. > > > > > > > > > > > > > > Sorry, not sure I understand clearly here. > > > > > > > > > > > > > > This the dts file ie used matched compatible. > > > > > > > compatible = "engicam,icore-mx8mm-ctouch2", "engicam,icore-mx8mm", > > > > > > > "fsl,imx8mm"; > > > > > > > > > > > > > > I did build the dtbs_check without showing any issues like, > > > > > > > > > > > > > > $ make ARCH=arm64 dtbs_check > > > > > > > ... > > > > > > > > > > > > > > From schema: > > > > > > > /w/dt-schema/dt-schema/dtschema/schemas/property-units.yaml > > > > > > > DTC > > > > > > > arch/arm64/boot/dts/freescale/imx8mm-icore-mx8mm-ctouch2.dtb > > > > > > > DTC > > > > > > > arch/arm64/boot/dts/freescale/imx8mm-icore-mx8mm-ctouch2-of10.dtb > > > > > > > DTC > > > > > > > arch/arm64/boo
Re: [PATCH v2 2/6] dt-bindings: arm: fsl: Add Engicam i.Core MX8M Mini C.TOUCH 2.0
On Tue, 22 Dec 2020 at 19:28, Jagan Teki wrote: > > On Mon, Dec 21, 2020 at 8:17 PM Jagan Teki wrote: > > > > On Mon, Dec 21, 2020 at 8:12 PM Krzysztof Kozlowski wrote: > > > > > > On Mon, Dec 21, 2020 at 08:09:47PM +0530, Jagan Teki wrote: > > > > On Mon, Dec 21, 2020 at 7:35 PM Krzysztof Kozlowski > > > > wrote: > > > > > > > > > > On Mon, Dec 21, 2020 at 07:29:22PM +0530, Jagan Teki wrote: > > > > > > On Mon, Dec 21, 2020 at 7:16 PM Krzysztof Kozlowski > > > > > > wrote: > > > > > > > > > > > > > > On Mon, Dec 21, 2020 at 05:01:47PM +0530, Jagan Teki wrote: > > > > > > > > i.Core MX8M Mini is an EDIMM SoM based on NXP i.MX8M Mini from > > > > > > > > Engicam. > > > > > > > > > > > > > > > > C.TOUCH 2.0 is a general purpose carrier board with capacitive > > > > > > > > touch interface support. > > > > > > > > > > > > > > > > i.Core MX8M Mini needs to mount on top of this Carrier board for > > > > > > > > creating complete i.Core MX8M Mini C.TOUCH 2.0 board. > > > > > > > > > > > > > > > > Add bindings for it. > > > > > > > > > > > > > > > > Signed-off-by: Jagan Teki > > > > > > > > --- > > > > > > > > Changes for v2: > > > > > > > > - updated commit message > > > > > > > > > > > > > > > > Documentation/devicetree/bindings/arm/fsl.yaml | 2 ++ > > > > > > > > 1 file changed, 2 insertions(+) > > > > > > > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml > > > > > > > > b/Documentation/devicetree/bindings/arm/fsl.yaml > > > > > > > > index 67980dcef66d..e653e0a43016 100644 > > > > > > > > --- a/Documentation/devicetree/bindings/arm/fsl.yaml > > > > > > > > +++ b/Documentation/devicetree/bindings/arm/fsl.yaml > > > > > > > > @@ -667,6 +667,8 @@ properties: > > > > > > > > items: > > > > > > > >- enum: > > > > > > > >- beacon,imx8mm-beacon-kit # i.MX8MM Beacon > > > > > > > > Development Kit > > > > > > > > + - engicam,icore-mx8mm # i.MX8MM > > > > > > > > Engicam i.Core MX8M Mini SOM > > > > > > > > + - engicam,icore-mx8mm-ctouch2 # i.MX8MM > > > > > > > > Engicam i.Core MX8M Mini C.TOUCH 2.0 > > > > > > > > > > > > > > Please test your DTS against new schema with dtbs_check. This > > > > > > > won't > > > > > > > match. > > > > > > > > > > > > Sorry, not sure I understand clearly here. > > > > > > > > > > > > This the dts file ie used matched compatible. > > > > > > compatible = "engicam,icore-mx8mm-ctouch2", "engicam,icore-mx8mm", > > > > > > "fsl,imx8mm"; > > > > > > > > > > > > I did build the dtbs_check without showing any issues like, > > > > > > > > > > > > $ make ARCH=arm64 dtbs_check > > > > > > ... > > > > > > > > > > > > From schema: > > > > > > /w/dt-schema/dt-schema/dtschema/schemas/property-units.yaml > > > > > > DTC > > > > > > arch/arm64/boot/dts/freescale/imx8mm-icore-mx8mm-ctouch2.dtb > > > > > > DTC > > > > > > arch/arm64/boot/dts/freescale/imx8mm-icore-mx8mm-ctouch2-of10.dtb > > > > > > DTC > > > > > > arch/arm64/boot/dts/freescale/imx8mm-icore-mx8mm-edimm2.2.dtb > > > > > > .. > > > > > > > > > > > > Can you let me know what I missed here? > > > > > > > > > > You pasted here output of validating with property-units.yaml (or > > > > > something else), not the schema which you changed. If you want to > > > > > limit > > > > > the tests, use DT_SCHEMA_FILES. > > > > >
Re: [PATCH v2 6/6] arm64: dts: imx8mm: Add Engicam i.Core MX8M Mini EDIMM2.2 Starter Kit
On Tue, Dec 22, 2020 at 02:20:55PM +0530, Jagan Teki wrote: > On Tue, Dec 22, 2020 at 2:36 AM Krzysztof Kozlowski wrote: > > > > On Tue, Dec 22, 2020 at 01:03:07AM +0530, Jagan Teki wrote: > > > On Mon, Dec 21, 2020 at 7:36 PM Krzysztof Kozlowski > > > wrote: > > > > > > > > On Mon, Dec 21, 2020 at 05:01:51PM +0530, Jagan Teki wrote: > > > > > Engicam EDIMM2.2 Starter Kit is an EDIMM 2.2 Form Factor Capacitive > > > > > Evaluation Board. > > > > > > > > > > Genaral features: > > > > > - LCD 7" C.Touch > > > > > - microSD slot > > > > > - Ethernet 1Gb > > > > > - Wifi/BT > > > > > - 2x LVDS Full HD interfaces > > > > > - 3x USB 2.0 > > > > > - 1x USB 3.0 > > > > > - HDMI Out > > > > > - Mini PCIe > > > > > - MIPI CSI > > > > > - 2x CAN > > > > > - Audio Out > > > > > > > > > > i.Core MX8M Mini is an EDIMM SoM based on NXP i.MX8M Mini from > > > > > Engicam. > > > > > > > > > > i.Core MX8M Mini needs to mount on top of this Evaluation board for > > > > > creating complete i.Core MX8M Mini EDIMM2.2 Starter Kit. > > > > > > > > > > PCIe, DSI, CSI nodes will add it into imx8mm-engicam-edimm2.2.dtsi > > > > > once > > > > > Mainline Linux supported. > > > > > > > > > > Add support for it. > > > > > > > > > > Signed-off-by: Matteo Lisi > > > > > Signed-off-by: Jagan Teki > > > > > --- > > > > > Changes for v2: > > > > > - updated commit message > > > > > - dropped engicam from filename since it aligned with imx6 engicam > > > > > dts files naming conventions. > > > > > > > > > > arch/arm64/boot/dts/freescale/Makefile| 1 + > > > > > .../freescale/imx8mm-engicam-edimm2.2.dtsi| 7 +++ > > > > > .../freescale/imx8mm-icore-mx8mm-edimm2.2.dts | 21 > > > > > +++ > > > > > 3 files changed, 29 insertions(+) > > > > > create mode 100644 > > > > > arch/arm64/boot/dts/freescale/imx8mm-engicam-edimm2.2.dtsi > > > > > create mode 100644 > > > > > arch/arm64/boot/dts/freescale/imx8mm-icore-mx8mm-edimm2.2.dts > > > > > > > > > > diff --git a/arch/arm64/boot/dts/freescale/Makefile > > > > > b/arch/arm64/boot/dts/freescale/Makefile > > > > > index 8d49a2c74604..43783076f856 100644 > > > > > --- a/arch/arm64/boot/dts/freescale/Makefile > > > > > +++ b/arch/arm64/boot/dts/freescale/Makefile > > > > > @@ -33,6 +33,7 @@ dtb-$(CONFIG_ARCH_MXC) += imx8mm-beacon-kit.dtb > > > > > dtb-$(CONFIG_ARCH_MXC) += imx8mm-evk.dtb > > > > > dtb-$(CONFIG_ARCH_MXC) += imx8mm-ddr4-evk.dtb > > > > > dtb-$(CONFIG_ARCH_MXC) += imx8mm-icore-mx8mm-ctouch2.dtb > > > > > +dtb-$(CONFIG_ARCH_MXC) += imx8mm-icore-mx8mm-edimm2.2.dtb > > > > > dtb-$(CONFIG_ARCH_MXC) += imx8mm-kontron-n801x-s.dtb > > > > > dtb-$(CONFIG_ARCH_MXC) += imx8mm-var-som-symphony.dtb > > > > > dtb-$(CONFIG_ARCH_MXC) += imx8mn-evk.dtb > > > > > diff --git > > > > > a/arch/arm64/boot/dts/freescale/imx8mm-engicam-edimm2.2.dtsi > > > > > b/arch/arm64/boot/dts/freescale/imx8mm-engicam-edimm2.2.dtsi > > > > > new file mode 100644 > > > > > index ..294df07289a2 > > > > > --- /dev/null > > > > > +++ b/arch/arm64/boot/dts/freescale/imx8mm-engicam-edimm2.2.dtsi > > > > > @@ -0,0 +1,7 @@ > > > > > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) > > > > > +/* > > > > > + * Copyright (c) 2020 Engicam srl > > > > > + * Copyright (c) 2020 Amarula Solutions(India) > > > > > + */ > > > > > + > > > > > +#include "imx8mm-engicam-common.dtsi" > > > > > > > > It seems you ignored my comments from previous email. That's not how we > > > > go with the process. > > > > > > > > Don't create confusing or overcomplicated hierarchy of includes. Don't > > > > create files which do nothing. > > > > > > Idea is to move common nodes in separate dtsi instead of adding > > > redundant nodes into respective areas. let me know if it still > > > confusing. > > > > A file which *only* includes another file does not fulfill this idea of > > moving common nodes to a separate DTSI file. Or if I still miss > > something, please point me, what common nodes are stored in > > imx8mm-engicam-edimm2.2.dtsi? > > imx8mm-engicam-edimm2.2.dtsi for EDIMM2.2 Carrier > imx8mm-engicam-ctouch2.dtsi for C.TOUCH2 Carrier > imx8mm-engicam-common.dtsi for common nodes for above 2 carrier boards. > > Yes, imx8mm-engicam-edimm2.2.dtsi is empty now Then that's the answer. We do not create empty files. > but nodes like PCIe, > CSI, DSI will support once the respective drivers are part of Mainline > but those are not supported in C.TOUCH2 carrier board dtsi. There are > some GPIO pins differences between EDIMM2.2 and C.TOUCH2 carriers on > WiFi/BT so those will be part of the respective carrier dtsi. It's the same clear as before. Don't create empty files. Once you decide to bring new features, you create a new file. Best regards, Krzysztof
Re: [PATCH v3 4/7] power: supply: max8997_charger: Set CHARGER current limit
On Tue, Dec 22, 2020 at 07:31:40AM +, Timon Baetz wrote:
> Register for extcon notification and set charging current depending on
> the detected cable type. Current values are taken from vendor kernel,
> where most charger types end up setting 650mA [0].
>
> Also enable and disable the CHARGER regulator based on extcon events.
>
> [0]
> https://github.com/krzk/linux-vendor-backup/blob/samsung/galaxy-s2-epic-4g-touch-sph-d710-exynos4210-dump/drivers/misc/max8997-muic.c#L1675-L1678
>
> Signed-off-by: Timon Baetz
> ---
> drivers/power/supply/max8997_charger.c | 94 ++
> 1 file changed, 94 insertions(+)
>
> diff --git a/drivers/power/supply/max8997_charger.c
> b/drivers/power/supply/max8997_charger.c
> index 1947af25879a..67314ae0 100644
> --- a/drivers/power/supply/max8997_charger.c
> +++ b/drivers/power/supply/max8997_charger.c
> @@ -6,12 +6,14 @@
> // MyungJoo Ham
>
> #include
> +#include
> #include
> #include
> #include
> #include
> #include
> #include
> +#include
>
> /* MAX8997_REG_STATUS4 */
> #define DCINOK_SHIFT 1
> @@ -31,6 +33,10 @@ struct charger_data {
> struct device *dev;
> struct max8997_dev *iodev;
> struct power_supply *battery;
> + struct regulator *reg;
> + struct extcon_dev *edev;
> + struct notifier_block extcon_nb;
> + struct work_struct extcon_work;
> };
>
> static enum power_supply_property max8997_battery_props[] = {
> @@ -88,6 +94,67 @@ static int max8997_battery_get_property(struct
> power_supply *psy,
> return 0;
> }
>
> +static void max8997_battery_extcon_evt_stop_work(void *data)
> +{
> + struct charger_data *charger = data;
> +
> + cancel_work_sync(>extcon_work);
> +}
> +
> +static void max8997_battery_extcon_evt_worker(struct work_struct *work)
> +{
> + struct charger_data *charger =
> + container_of(work, struct charger_data, extcon_work);
> + struct extcon_dev *edev = charger->edev;
> + int current_limit;
> +
> + if (extcon_get_state(edev, EXTCON_CHG_USB_SDP) > 0) {
> + dev_dbg(charger->dev, "USB SDP charger is connected\n");
> + current_limit = 45;
> + } else if (extcon_get_state(edev, EXTCON_CHG_USB_DCP) > 0) {
> + dev_dbg(charger->dev, "USB DCP charger is connected\n");
> + current_limit = 65;
> + } else if (extcon_get_state(edev, EXTCON_CHG_USB_FAST) > 0) {
> + dev_dbg(charger->dev, "USB FAST charger is connected\n");
> + current_limit = 65;
> + } else if (extcon_get_state(edev, EXTCON_CHG_USB_SLOW) > 0) {
> + dev_dbg(charger->dev, "USB SLOW charger is connected\n");
> + current_limit = 65;
> + } else if (extcon_get_state(edev, EXTCON_CHG_USB_CDP) > 0) {
> + dev_dbg(charger->dev, "USB CDP charger is connected\n");
> + current_limit = 65;
> + } else {
> + dev_dbg(charger->dev, "USB charger is diconnected\n");
> + current_limit = -1;
> + }
> +
> + if (current_limit > 0) {
> + int ret = regulator_set_current_limit(charger->reg,
> current_limit, current_limit);
> +
> + if (ret) {
> + dev_err(charger->dev, "failed to set current limit:
> %d\n", ret);
> + return;
> + }
> + ret = regulator_enable(charger->reg);
> + if (ret)
> + dev_err(charger->dev, "failed to enable regulator:
> %d\n", ret);
> + } else {
> + int ret = regulator_disable(charger->reg);
> +
> + if (ret)
> + dev_err(charger->dev, "failed to disable regulator:
> %d\n", ret);
> + }
> +}
> +
> +static int max8997_battery_extcon_evt(struct notifier_block *nb,
> + unsigned long event, void *param)
> +{
> + struct charger_data *charger =
> + container_of(nb, struct charger_data, extcon_nb);
> + schedule_work(>extcon_work);
> + return NOTIFY_OK;
> +}
> +
> static const struct power_supply_desc max8997_battery_desc = {
> .name = "max8997_pmic",
> .type = POWER_SUPPLY_TYPE_BATTERY,
> @@ -170,6 +237,33 @@ static int max8997_battery_probe(struct platform_device
> *pdev)
> return PTR_ERR(charger->battery);
> }
>
> + charger->reg = devm_regulator_get(>dev, "charger");
The code looks good but isn't it breaking all existing platforms?
Best regards,
Krzysztof
> + if (IS_ERR(charger->reg)) {
> + dev_err(>dev, "couldn't get charger regulator\n");
> + return PTR_ERR(charger->reg);
> + }
> +
Re: [PATCH v3 3/7] mfd: max8997: Add of_compatible to extcon and charger mfd_cell
On Tue, Dec 22, 2020 at 07:31:32AM +, Timon Baetz wrote:
> Add of_compatible ("maxim,max8997-muic") to the mfd_cell to have a
> of_node set in the extcon driver.
>
> Add of_compatible ("maxim,max8997-battery") to the mfd_cell to configure
> the charger driver.
>
> Signed-off-by: Timon Baetz
> ---
> drivers/mfd/max8997.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: Krzysztof Kozlowski
Best regards,
Krzysztof
Re: [PATCH v3 2/7] regulator: dt-bindings: Document max8997-pmic nodes
On Tue, Dec 22, 2020 at 07:31:21AM +, Timon Baetz wrote: > Add maxim,max8997-battery and maxim,max8997-muic optional nodes. > > Signed-off-by: Timon Baetz > --- > .../bindings/regulator/max8997-regulator.txt | 11 +++ > 1 file changed, 11 insertions(+) > Acked-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH v9 1/4] dt-bindings: soc: imx8m: add DT Binding doc for soc unique ID
On Tue, Dec 22, 2020 at 04:10:54PM +0800, Alice Guo (OSS) wrote: > From: Alice Guo > > Add DT Binding doc for the Unique ID of i.MX 8M series. > > Signed-off-by: Alice Guo > --- > > Changes for v9: > - add additionalProperties for "^soc@[0-9a-f]+$" > - add examples > Changes for v8: > - match soc node with regular expression > Changes for v7: > - change to a separate schema file > Changes for v6: > - none > Changes for v5: > - correct the error of using allOf > Changes for v4: > - use allOf to limit new version DTS files for i.MX8M to include >"fsl,imx8m*-soc", nvmem-cells and nvmem-cells-names > Changes for v3: > - put it into Documentation/devicetree/bindings/arm/fsl.yaml > - modify the description of nvmem-cells > - use "make ARCH=arm64 dtbs_check" to make sure it is right > Changes for v2: > - remove the subject prefix "LF-2571-1" > > .../bindings/soc/imx/imx8m-soc.yaml | 86 +++ > 1 file changed, 86 insertions(+) > create mode 100644 Documentation/devicetree/bindings/soc/imx/imx8m-soc.yaml > Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH v2 6/6] arm64: dts: imx8mm: Add Engicam i.Core MX8M Mini EDIMM2.2 Starter Kit
On Tue, Dec 22, 2020 at 01:03:07AM +0530, Jagan Teki wrote: > On Mon, Dec 21, 2020 at 7:36 PM Krzysztof Kozlowski wrote: > > > > On Mon, Dec 21, 2020 at 05:01:51PM +0530, Jagan Teki wrote: > > > Engicam EDIMM2.2 Starter Kit is an EDIMM 2.2 Form Factor Capacitive > > > Evaluation Board. > > > > > > Genaral features: > > > - LCD 7" C.Touch > > > - microSD slot > > > - Ethernet 1Gb > > > - Wifi/BT > > > - 2x LVDS Full HD interfaces > > > - 3x USB 2.0 > > > - 1x USB 3.0 > > > - HDMI Out > > > - Mini PCIe > > > - MIPI CSI > > > - 2x CAN > > > - Audio Out > > > > > > i.Core MX8M Mini is an EDIMM SoM based on NXP i.MX8M Mini from Engicam. > > > > > > i.Core MX8M Mini needs to mount on top of this Evaluation board for > > > creating complete i.Core MX8M Mini EDIMM2.2 Starter Kit. > > > > > > PCIe, DSI, CSI nodes will add it into imx8mm-engicam-edimm2.2.dtsi once > > > Mainline Linux supported. > > > > > > Add support for it. > > > > > > Signed-off-by: Matteo Lisi > > > Signed-off-by: Jagan Teki > > > --- > > > Changes for v2: > > > - updated commit message > > > - dropped engicam from filename since it aligned with imx6 engicam > > > dts files naming conventions. > > > > > > arch/arm64/boot/dts/freescale/Makefile| 1 + > > > .../freescale/imx8mm-engicam-edimm2.2.dtsi| 7 +++ > > > .../freescale/imx8mm-icore-mx8mm-edimm2.2.dts | 21 +++ > > > 3 files changed, 29 insertions(+) > > > create mode 100644 > > > arch/arm64/boot/dts/freescale/imx8mm-engicam-edimm2.2.dtsi > > > create mode 100644 > > > arch/arm64/boot/dts/freescale/imx8mm-icore-mx8mm-edimm2.2.dts > > > > > > diff --git a/arch/arm64/boot/dts/freescale/Makefile > > > b/arch/arm64/boot/dts/freescale/Makefile > > > index 8d49a2c74604..43783076f856 100644 > > > --- a/arch/arm64/boot/dts/freescale/Makefile > > > +++ b/arch/arm64/boot/dts/freescale/Makefile > > > @@ -33,6 +33,7 @@ dtb-$(CONFIG_ARCH_MXC) += imx8mm-beacon-kit.dtb > > > dtb-$(CONFIG_ARCH_MXC) += imx8mm-evk.dtb > > > dtb-$(CONFIG_ARCH_MXC) += imx8mm-ddr4-evk.dtb > > > dtb-$(CONFIG_ARCH_MXC) += imx8mm-icore-mx8mm-ctouch2.dtb > > > +dtb-$(CONFIG_ARCH_MXC) += imx8mm-icore-mx8mm-edimm2.2.dtb > > > dtb-$(CONFIG_ARCH_MXC) += imx8mm-kontron-n801x-s.dtb > > > dtb-$(CONFIG_ARCH_MXC) += imx8mm-var-som-symphony.dtb > > > dtb-$(CONFIG_ARCH_MXC) += imx8mn-evk.dtb > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mm-engicam-edimm2.2.dtsi > > > b/arch/arm64/boot/dts/freescale/imx8mm-engicam-edimm2.2.dtsi > > > new file mode 100644 > > > index ..294df07289a2 > > > --- /dev/null > > > +++ b/arch/arm64/boot/dts/freescale/imx8mm-engicam-edimm2.2.dtsi > > > @@ -0,0 +1,7 @@ > > > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) > > > +/* > > > + * Copyright (c) 2020 Engicam srl > > > + * Copyright (c) 2020 Amarula Solutions(India) > > > + */ > > > + > > > +#include "imx8mm-engicam-common.dtsi" > > > > It seems you ignored my comments from previous email. That's not how we > > go with the process. > > > > Don't create confusing or overcomplicated hierarchy of includes. Don't > > create files which do nothing. > > Idea is to move common nodes in separate dtsi instead of adding > redundant nodes into respective areas. let me know if it still > confusing. A file which *only* includes another file does not fulfill this idea of moving common nodes to a separate DTSI file. Or if I still miss something, please point me, what common nodes are stored in imx8mm-engicam-edimm2.2.dtsi? Best regards, Krzysztof
Re: [PATCH v2 16/18] arm64: dts: hi3660: Harmonize DWC USB3 DT nodes name
On Mon, Dec 21, 2020 at 12:24:11PM -0800, John Stultz wrote:
> On Sat, Dec 19, 2020 at 3:06 AM Krzysztof Kozlowski wrote:
> > On Fri, Dec 18, 2020 at 09:11:42PM -0800, John Stultz wrote:
> > > On Wed, Nov 11, 2020 at 1:22 AM Serge Semin
> > > wrote:
> > > >
> > > > In accordance with the DWC USB3 bindings the corresponding node
> > > > name is suppose to comply with the Generic USB HCD DT schema, which
> > > > requires the USB nodes to have the name acceptable by the regexp:
> > > > "^usb(@.*)?" . Make sure the "snps,dwc3"-compatible nodes are correctly
> > > > named.
> > > >
> > > > Signed-off-by: Serge Semin
> > > > Acked-by: Krzysztof Kozlowski
> > > > ---
> > > > arch/arm64/boot/dts/hisilicon/hi3660.dtsi | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
> > > > b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
> > > > index d25aac5e0bf8..aea3800029b5 100644
> > > > --- a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
> > > > +++ b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
> > > > @@ -1166,7 +1166,7 @@ usb_phy: usb-phy {
> > > > };
> > > > };
> > > >
> > > > - dwc3: dwc3@ff10 {
> > > > + dwc3: usb@ff10 {
> > > > compatible = "snps,dwc3";
> > > > reg = <0x0 0xff10 0x0 0x10>;
> > >
> > >
> > > Oof. So this patch is breaking the usb gadget functionality on HiKey960
> > > w/ AOSP.
> > >
> > > In order to choose the right controller for gadget mode with AOSP, one
> > > sets the "sys.usb.controller" property, which until now for HiKey960
> > > has been "ff10.dwc3".
> > > After this patch, the controller isn't found and we would have to
> > > change userland to use "ff10.usb", which would then break booting
> > > on older kernels (testing various LTS releases on AOSP is one of the
> > > key uses of the HiKey960).
> > >
> > > So while I understand the desire to unify the schema, as HiKey960
> > > really isn't likely to be used outside of AOSP, I wonder if reverting
> > > this one change is in the best interest of not breaking existing
> > > userland?
> >
> > The node names are not part of an ABI, are they? I expect only
> > compatibles and properties to be stable. If user-space looks for
> > something by name, it's a user-space's mistake. Not mentioning that you
> > also look for specific address... Imagine remapping of addresses with
> > ranges (for whatever reason) - AOSP also would be broken? Addresses are
> > definitely not an ABI.
>
> Though that is how it's exported through sysfs.
The ABI is the format of sysfs file for example in /sys/devices. However
the ABI is not the exact address or node name of each device.
> In AOSP it is then used to setup the configfs gadget by writing that
> value into /config/usb_gadget/g1/UDC.
>
> Given there may be multiple controllers on a device, or even if its
> just one and the dummy hcd driver is enabled, I'm not sure how folks
> reference the "right" one without the node name?
I think it is the same type of problem as for all other subsystems, e.g.
mmc, hwmon/iio. They usually solve it either with aliases or with
special property with the name/label.
> I understand the fuzziness with sysfs ABI, and I get that having
> consistent naming is important, but like the eth0 -> enp3s0 changes,
> it seems like this is going to break things.
One could argue whether interface name is or is not ABI. But please tell
me how the address of a device in one's representation (for example DT)
is a part of a stable interface?
> Greg? Is there some better way AOSP should be doing this?
If you need to find specific device, maybe go through the given bus and
check compatibles?
Best regards,
Krzysztof
Re: [PATCH v2 2/6] power: supply: max8997_charger: Set CHARGER current limit
On Mon, Dec 21, 2020 at 03:35:07PM +, Timon Baetz wrote:
> On Mon, 21 Dec 2020 15:16:27 +0100, Krzysztof Kozlowski wrote:
> > On Mon, Dec 21, 2020 at 09:53:15AM +, Timon Baetz wrote:
> > > Register for extcon notification and set charging current depending on
> > > the detected cable type. Current values are taken from vendor kernel,
> > > where most charger types end up setting 650mA [0].
> > >
> > > Also enable and disable the CHARGER regulator based on extcon events.
> > >
> > > [0]
> > > https://github.com/krzk/linux-vendor-backup/blob/samsung/galaxy-s2-epic-4g-touch-sph-d710-exynos4210-dump/drivers/misc/max8997-muic.c#L1675-L1678
> > >
> > > Signed-off-by: Timon Baetz
> > > ---
> > > drivers/mfd/max8997.c | 4 +-
> > > drivers/power/supply/max8997_charger.c | 94 ++
> > > 2 files changed, 96 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/mfd/max8997.c b/drivers/mfd/max8997.c
> > > index 68d8f2b95287..55d3a6f97783 100644
> > > --- a/drivers/mfd/max8997.c
> > > +++ b/drivers/mfd/max8997.c
> > > @@ -29,9 +29,9 @@
> > > static const struct mfd_cell max8997_devs[] = {
> > > { .name = "max8997-pmic", },
> > > { .name = "max8997-rtc", },
> > > - { .name = "max8997-battery", },
> > > + { .name = "max8997-battery", .of_compatible = "maxim,max8997-battery",
> > > },
> > > { .name = "max8997-haptic", },
> > > - { .name = "max8997-muic", },
> > > + { .name = "max8997-muic", .of_compatible = "maxim,max8997-muic", },
> >
> > Undocumented bindings. The checkpatch should complain about it, so I
> > assume you did not run it. Please run the checkpatch.
> >
> > > { .name = "max8997-led", .id = 1 },
> > > { .name = "max8997-led", .id = 2 },
> > > };
> > > diff --git a/drivers/power/supply/max8997_charger.c
> > > b/drivers/power/supply/max8997_charger.c
> > > index 1947af25879a..6e8750e455ea 100644
> > > --- a/drivers/power/supply/max8997_charger.c
> > > +++ b/drivers/power/supply/max8997_charger.c
> > > @@ -6,12 +6,14 @@
> > > // MyungJoo Ham
> > >
> > > #include
> > > +#include
> > > #include
> > > #include
> > > #include
> > > #include
> > > #include
> > > #include
> > > +#include
> > >
> > > /* MAX8997_REG_STATUS4 */
> > > #define DCINOK_SHIFT 1
> > > @@ -31,6 +33,10 @@ struct charger_data {
> > > struct device *dev;
> > > struct max8997_dev *iodev;
> > > struct power_supply *battery;
> > > + struct regulator *reg;
> > > + struct extcon_dev *edev;
> > > + struct notifier_block extcon_nb;
> > > + struct work_struct extcon_work;
> > > };
> > >
> > > static enum power_supply_property max8997_battery_props[] = {
> > > @@ -88,6 +94,67 @@ static int max8997_battery_get_property(struct
> > > power_supply *psy,
> > > return 0;
> > > }
> > >
> > > +static void max8997_battery_extcon_evt_stop_work(void *data)
> > > +{
> > > + struct charger_data *charger = data;
> > > +
> > > + cancel_work_sync(>extcon_work);
> > > +}
> > > +
> > > +static void max8997_battery_extcon_evt_worker(struct work_struct *work)
> > > +{
> > > + struct charger_data *charger =
> > > + container_of(work, struct charger_data, extcon_work);
> > > + struct extcon_dev *edev = charger->edev;
> > > + int current_limit, ret;
> > > +
> > > + if (extcon_get_state(edev, EXTCON_CHG_USB_SDP) > 0) {
> > > + dev_dbg(charger->dev, "USB SDP charger is connected\n");
> > > + current_limit = 45;
> > > + } else if (extcon_get_state(edev, EXTCON_CHG_USB_DCP) > 0) {
> > > + dev_dbg(charger->dev, "USB DCP charger is connected\n");
> > > + current_limit = 65;
> > > + } else if (extcon_get_state(edev, EXTCON_CHG_USB_FAST) > 0) {
> > > + dev_dbg(charger->dev, "USB FAST charger is connected\n");
> > > + current_limit = 65;
> > > + } else if (extcon_get_state(edev, EXTCON_CHG_USB_SLOW) > 0) {
> > > + dev_dbg(charger->dev, "USB SLOW charger is connecte
Re: [PATCH v2 2/6] dt-bindings: arm: fsl: Add Engicam i.Core MX8M Mini C.TOUCH 2.0
On Mon, Dec 21, 2020 at 08:09:47PM +0530, Jagan Teki wrote: > On Mon, Dec 21, 2020 at 7:35 PM Krzysztof Kozlowski wrote: > > > > On Mon, Dec 21, 2020 at 07:29:22PM +0530, Jagan Teki wrote: > > > On Mon, Dec 21, 2020 at 7:16 PM Krzysztof Kozlowski > > > wrote: > > > > > > > > On Mon, Dec 21, 2020 at 05:01:47PM +0530, Jagan Teki wrote: > > > > > i.Core MX8M Mini is an EDIMM SoM based on NXP i.MX8M Mini from > > > > > Engicam. > > > > > > > > > > C.TOUCH 2.0 is a general purpose carrier board with capacitive > > > > > touch interface support. > > > > > > > > > > i.Core MX8M Mini needs to mount on top of this Carrier board for > > > > > creating complete i.Core MX8M Mini C.TOUCH 2.0 board. > > > > > > > > > > Add bindings for it. > > > > > > > > > > Signed-off-by: Jagan Teki > > > > > --- > > > > > Changes for v2: > > > > > - updated commit message > > > > > > > > > > Documentation/devicetree/bindings/arm/fsl.yaml | 2 ++ > > > > > 1 file changed, 2 insertions(+) > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml > > > > > b/Documentation/devicetree/bindings/arm/fsl.yaml > > > > > index 67980dcef66d..e653e0a43016 100644 > > > > > --- a/Documentation/devicetree/bindings/arm/fsl.yaml > > > > > +++ b/Documentation/devicetree/bindings/arm/fsl.yaml > > > > > @@ -667,6 +667,8 @@ properties: > > > > > items: > > > > >- enum: > > > > >- beacon,imx8mm-beacon-kit # i.MX8MM Beacon > > > > > Development Kit > > > > > + - engicam,icore-mx8mm # i.MX8MM Engicam > > > > > i.Core MX8M Mini SOM > > > > > + - engicam,icore-mx8mm-ctouch2 # i.MX8MM Engicam > > > > > i.Core MX8M Mini C.TOUCH 2.0 > > > > > > > > Please test your DTS against new schema with dtbs_check. This won't > > > > match. > > > > > > Sorry, not sure I understand clearly here. > > > > > > This the dts file ie used matched compatible. > > > compatible = "engicam,icore-mx8mm-ctouch2", "engicam,icore-mx8mm", > > > "fsl,imx8mm"; > > > > > > I did build the dtbs_check without showing any issues like, > > > > > > $ make ARCH=arm64 dtbs_check > > > ... > > > > > > From schema: > > > /w/dt-schema/dt-schema/dtschema/schemas/property-units.yaml > > > DTC arch/arm64/boot/dts/freescale/imx8mm-icore-mx8mm-ctouch2.dtb > > > DTC > > > arch/arm64/boot/dts/freescale/imx8mm-icore-mx8mm-ctouch2-of10.dtb > > > DTC arch/arm64/boot/dts/freescale/imx8mm-icore-mx8mm-edimm2.2.dtb > > > .. > > > > > > Can you let me know what I missed here? > > > > You pasted here output of validating with property-units.yaml (or > > something else), not the schema which you changed. If you want to limit > > the tests, use DT_SCHEMA_FILES. > > > > I mentioned about exactly the same problem in yout previous v1 > > at patch #5. No changes here stil. > > Yes, I usually did that check before posting. Please check the build > log below and fsl.yaml binding is fine to build. > > # make dt_binding_check DT_SCHEMA_FILES=arm/fsl.yaml 1. Wrong path to schema file, 2. Bindings pass, they are not a problem. You were running dtbs_check, right? Best regards, Krzysztof
Re: [PATCH v2 6/6] regulator: dt-bindings: Document max8997-pmic nodes
On Mon, Dec 21, 2020 at 09:53:41AM +, Timon Baetz wrote: > Add maxim,max8997-battery and maxim,max8997-muic optional nodes. > > Signed-off-by: Timon Baetz > --- > .../bindings/regulator/max8997-regulator.txt | 11 +++ > 1 file changed, 11 insertions(+) I see you updated the bindings. However if you run scripts/checkpatch, it would say that bindings go as first patch, so please re-order. Best regards, Krzysztof
Re: [PATCH v2 5/6] ARM: dts: exynos: Added top-off charging regulator node for i9100
On Mon, Dec 21, 2020 at 09:53:35AM +, Timon Baetz wrote:
> Value taken from Galaxy S2 vendor kernel [0] which always sets 200mA.
Subject: "Add", not "Added", as in Linux coding style.
>
> Also rearrange regulators based on definition in max8997.h.
>
> [0]
> https://github.com/krzk/linux-vendor-backup/blob/samsung/galaxy-s2-epic-4g-touch-sph-d710-exynos4210-dump/drivers/power/sec_battery_u1.c#L1525
>
> Signed-off-by: Timon Baetz
> ---
> arch/arm/boot/dts/exynos4210-i9100.dts | 21 ++---
> 1 file changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/arch/arm/boot/dts/exynos4210-i9100.dts
> b/arch/arm/boot/dts/exynos4210-i9100.dts
> index 586d801af0b5..fec6da64f7c1 100644
> --- a/arch/arm/boot/dts/exynos4210-i9100.dts
> +++ b/arch/arm/boot/dts/exynos4210-i9100.dts
> @@ -560,6 +560,16 @@ safe2_sreg: ESAFEOUT2 {
> regulator-boot-on;
> };
>
> + EN32KHZ_AP {
> + regulator-name = "EN32KHZ_AP";
> + regulator-always-on;
> + };
> +
> + EN32KHZ_CP {
> + regulator-name = "EN32KHZ_CP";
> + regulator-always-on;
> + };
> +
> charger_reg: CHARGER {
> regulator-name = "CHARGER";
> regulator-min-microamp = <20>;
> @@ -573,13 +583,10 @@ chargercv_reg: CHARGER_CV {
> regulator-always-on;
> };
>
> - EN32KHZ_AP {
> - regulator-name = "EN32KHZ_AP";
> - regulator-always-on;
> - };
> -
> - EN32KHZ_CP {
> - regulator-name = "EN32KHZ_CP";
> + chargertopoff_reg: CHARGER_TOPOFF {
No need for label "chargertopoff_reg".
Best regards,
Krzysztof
Re: [PATCH v2 4/6] ARM: dts: exynos: Added muic and charger nodes for i9100
On Mon, Dec 21, 2020 at 09:53:28AM +, Timon Baetz wrote: > muic node is only used for extcon consumers. > charger node is used to specify muic and regulator. > > Signed-off-by: Timon Baetz > --- > arch/arm/boot/dts/exynos4210-i9100.dts | 10 ++ > 1 file changed, 10 insertions(+) Arrange your patches within the patchset in a way preserving bisectability. If 3/7 is applied, the charger will be off because kernel disables unused regulators. Best regards, Krzysztof
Re: [PATCH v2 3/6] ARM: dts: exynos: Fix charging regulator voltage and current for i9100
On Mon, Dec 21, 2020 at 09:53:22AM +, Timon Baetz wrote: > Set CHARGER current and CHARGER_CV voltage according to Galaxy S2 vendor > sources [0,1]. Mention that the vendor sources are for Galaxy S2 Epic 4G Touch SPH-D710 Android. This seems to depend on driver changes, so it will have to wait till they reach mainline. Best regards, Krzysztof
Re: [PATCH v2 2/6] power: supply: max8997_charger: Set CHARGER current limit
On Mon, Dec 21, 2020 at 09:53:15AM +, Timon Baetz wrote:
> Register for extcon notification and set charging current depending on
> the detected cable type. Current values are taken from vendor kernel,
> where most charger types end up setting 650mA [0].
>
> Also enable and disable the CHARGER regulator based on extcon events.
>
> [0]
> https://github.com/krzk/linux-vendor-backup/blob/samsung/galaxy-s2-epic-4g-touch-sph-d710-exynos4210-dump/drivers/misc/max8997-muic.c#L1675-L1678
>
> Signed-off-by: Timon Baetz
> ---
> drivers/mfd/max8997.c | 4 +-
> drivers/power/supply/max8997_charger.c | 94 ++
> 2 files changed, 96 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mfd/max8997.c b/drivers/mfd/max8997.c
> index 68d8f2b95287..55d3a6f97783 100644
> --- a/drivers/mfd/max8997.c
> +++ b/drivers/mfd/max8997.c
> @@ -29,9 +29,9 @@
> static const struct mfd_cell max8997_devs[] = {
> { .name = "max8997-pmic", },
> { .name = "max8997-rtc", },
> - { .name = "max8997-battery", },
> + { .name = "max8997-battery", .of_compatible = "maxim,max8997-battery",
> },
> { .name = "max8997-haptic", },
> - { .name = "max8997-muic", },
> + { .name = "max8997-muic", .of_compatible = "maxim,max8997-muic", },
Undocumented bindings. The checkpatch should complain about it, so I
assume you did not run it. Please run the checkpatch.
> { .name = "max8997-led", .id = 1 },
> { .name = "max8997-led", .id = 2 },
> };
> diff --git a/drivers/power/supply/max8997_charger.c
> b/drivers/power/supply/max8997_charger.c
> index 1947af25879a..6e8750e455ea 100644
> --- a/drivers/power/supply/max8997_charger.c
> +++ b/drivers/power/supply/max8997_charger.c
> @@ -6,12 +6,14 @@
> // MyungJoo Ham
>
> #include
> +#include
> #include
> #include
> #include
> #include
> #include
> #include
> +#include
>
> /* MAX8997_REG_STATUS4 */
> #define DCINOK_SHIFT 1
> @@ -31,6 +33,10 @@ struct charger_data {
> struct device *dev;
> struct max8997_dev *iodev;
> struct power_supply *battery;
> + struct regulator *reg;
> + struct extcon_dev *edev;
> + struct notifier_block extcon_nb;
> + struct work_struct extcon_work;
> };
>
> static enum power_supply_property max8997_battery_props[] = {
> @@ -88,6 +94,67 @@ static int max8997_battery_get_property(struct
> power_supply *psy,
> return 0;
> }
>
> +static void max8997_battery_extcon_evt_stop_work(void *data)
> +{
> + struct charger_data *charger = data;
> +
> + cancel_work_sync(>extcon_work);
> +}
> +
> +static void max8997_battery_extcon_evt_worker(struct work_struct *work)
> +{
> + struct charger_data *charger =
> + container_of(work, struct charger_data, extcon_work);
> + struct extcon_dev *edev = charger->edev;
> + int current_limit, ret;
> +
> + if (extcon_get_state(edev, EXTCON_CHG_USB_SDP) > 0) {
> + dev_dbg(charger->dev, "USB SDP charger is connected\n");
> + current_limit = 45;
> + } else if (extcon_get_state(edev, EXTCON_CHG_USB_DCP) > 0) {
> + dev_dbg(charger->dev, "USB DCP charger is connected\n");
> + current_limit = 65;
> + } else if (extcon_get_state(edev, EXTCON_CHG_USB_FAST) > 0) {
> + dev_dbg(charger->dev, "USB FAST charger is connected\n");
> + current_limit = 65;
> + } else if (extcon_get_state(edev, EXTCON_CHG_USB_SLOW) > 0) {
> + dev_dbg(charger->dev, "USB SLOW charger is connected\n");
> + current_limit = 65;
> + } else if (extcon_get_state(edev, EXTCON_CHG_USB_CDP) > 0) {
> + dev_dbg(charger->dev, "USB CDP charger is connected\n");
> + current_limit = 65;
> + } else {
> + dev_dbg(charger->dev, "USB charger is diconnected\n");
> + current_limit = -1;
> + }
> +
> + if (current_limit > 0) {
> + ret = regulator_set_current_limit(charger->reg, current_limit,
> current_limit);
> + if (ret) {
> + dev_err(charger->dev, "failed to set current limit:
> %d\n", ret);
> + goto regulator_disable;
Unusual error path... if regulator was not enabled before and
regulator_set_current_limit() failed, you disable the regulator? Why?
Wasn't it already disabled?
Best regards,
Krzysztof
Re: [PATCH v2 1/6] extcon: max8997: Add CHGINS and CHGRM interrupt handling
On Mon, Dec 21, 2020 at 09:53:08AM +, Timon Baetz wrote: > This allows the MAX8997 charger to set the current limit depending on > the detected extcon charger type. > > Signed-off-by: Timon Baetz Don't do this: In-Reply-To: <20201202203516.43053-1-timon.ba...@protonmail.com> It's a v2, so new thread. If you want to reference previous work, paste a link from lore.kernel.org. Best regards, Krzysztof
Re: [PATCH v2 6/6] arm64: dts: imx8mm: Add Engicam i.Core MX8M Mini EDIMM2.2 Starter Kit
On Mon, Dec 21, 2020 at 05:01:51PM +0530, Jagan Teki wrote: > Engicam EDIMM2.2 Starter Kit is an EDIMM 2.2 Form Factor Capacitive > Evaluation Board. > > Genaral features: > - LCD 7" C.Touch > - microSD slot > - Ethernet 1Gb > - Wifi/BT > - 2x LVDS Full HD interfaces > - 3x USB 2.0 > - 1x USB 3.0 > - HDMI Out > - Mini PCIe > - MIPI CSI > - 2x CAN > - Audio Out > > i.Core MX8M Mini is an EDIMM SoM based on NXP i.MX8M Mini from Engicam. > > i.Core MX8M Mini needs to mount on top of this Evaluation board for > creating complete i.Core MX8M Mini EDIMM2.2 Starter Kit. > > PCIe, DSI, CSI nodes will add it into imx8mm-engicam-edimm2.2.dtsi once > Mainline Linux supported. > > Add support for it. > > Signed-off-by: Matteo Lisi > Signed-off-by: Jagan Teki > --- > Changes for v2: > - updated commit message > - dropped engicam from filename since it aligned with imx6 engicam > dts files naming conventions. > > arch/arm64/boot/dts/freescale/Makefile| 1 + > .../freescale/imx8mm-engicam-edimm2.2.dtsi| 7 +++ > .../freescale/imx8mm-icore-mx8mm-edimm2.2.dts | 21 +++ > 3 files changed, 29 insertions(+) > create mode 100644 arch/arm64/boot/dts/freescale/imx8mm-engicam-edimm2.2.dtsi > create mode 100644 > arch/arm64/boot/dts/freescale/imx8mm-icore-mx8mm-edimm2.2.dts > > diff --git a/arch/arm64/boot/dts/freescale/Makefile > b/arch/arm64/boot/dts/freescale/Makefile > index 8d49a2c74604..43783076f856 100644 > --- a/arch/arm64/boot/dts/freescale/Makefile > +++ b/arch/arm64/boot/dts/freescale/Makefile > @@ -33,6 +33,7 @@ dtb-$(CONFIG_ARCH_MXC) += imx8mm-beacon-kit.dtb > dtb-$(CONFIG_ARCH_MXC) += imx8mm-evk.dtb > dtb-$(CONFIG_ARCH_MXC) += imx8mm-ddr4-evk.dtb > dtb-$(CONFIG_ARCH_MXC) += imx8mm-icore-mx8mm-ctouch2.dtb > +dtb-$(CONFIG_ARCH_MXC) += imx8mm-icore-mx8mm-edimm2.2.dtb > dtb-$(CONFIG_ARCH_MXC) += imx8mm-kontron-n801x-s.dtb > dtb-$(CONFIG_ARCH_MXC) += imx8mm-var-som-symphony.dtb > dtb-$(CONFIG_ARCH_MXC) += imx8mn-evk.dtb > diff --git a/arch/arm64/boot/dts/freescale/imx8mm-engicam-edimm2.2.dtsi > b/arch/arm64/boot/dts/freescale/imx8mm-engicam-edimm2.2.dtsi > new file mode 100644 > index ..294df07289a2 > --- /dev/null > +++ b/arch/arm64/boot/dts/freescale/imx8mm-engicam-edimm2.2.dtsi > @@ -0,0 +1,7 @@ > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) > +/* > + * Copyright (c) 2020 Engicam srl > + * Copyright (c) 2020 Amarula Solutions(India) > + */ > + > +#include "imx8mm-engicam-common.dtsi" It seems you ignored my comments from previous email. That's not how we go with the process. Don't create confusing or overcomplicated hierarchy of includes. Don't create files which do nothing. Best regards, Krzysztof
Re: [PATCH v2 2/6] dt-bindings: arm: fsl: Add Engicam i.Core MX8M Mini C.TOUCH 2.0
On Mon, Dec 21, 2020 at 07:29:22PM +0530, Jagan Teki wrote: > On Mon, Dec 21, 2020 at 7:16 PM Krzysztof Kozlowski wrote: > > > > On Mon, Dec 21, 2020 at 05:01:47PM +0530, Jagan Teki wrote: > > > i.Core MX8M Mini is an EDIMM SoM based on NXP i.MX8M Mini from Engicam. > > > > > > C.TOUCH 2.0 is a general purpose carrier board with capacitive > > > touch interface support. > > > > > > i.Core MX8M Mini needs to mount on top of this Carrier board for > > > creating complete i.Core MX8M Mini C.TOUCH 2.0 board. > > > > > > Add bindings for it. > > > > > > Signed-off-by: Jagan Teki > > > --- > > > Changes for v2: > > > - updated commit message > > > > > > Documentation/devicetree/bindings/arm/fsl.yaml | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml > > > b/Documentation/devicetree/bindings/arm/fsl.yaml > > > index 67980dcef66d..e653e0a43016 100644 > > > --- a/Documentation/devicetree/bindings/arm/fsl.yaml > > > +++ b/Documentation/devicetree/bindings/arm/fsl.yaml > > > @@ -667,6 +667,8 @@ properties: > > > items: > > >- enum: > > >- beacon,imx8mm-beacon-kit # i.MX8MM Beacon Development > > > Kit > > > + - engicam,icore-mx8mm # i.MX8MM Engicam > > > i.Core MX8M Mini SOM > > > + - engicam,icore-mx8mm-ctouch2 # i.MX8MM Engicam > > > i.Core MX8M Mini C.TOUCH 2.0 > > > > Please test your DTS against new schema with dtbs_check. This won't > > match. > > Sorry, not sure I understand clearly here. > > This the dts file ie used matched compatible. > compatible = "engicam,icore-mx8mm-ctouch2", "engicam,icore-mx8mm", > "fsl,imx8mm"; > > I did build the dtbs_check without showing any issues like, > > $ make ARCH=arm64 dtbs_check > ... > > From schema: /w/dt-schema/dt-schema/dtschema/schemas/property-units.yaml > DTC arch/arm64/boot/dts/freescale/imx8mm-icore-mx8mm-ctouch2.dtb > DTC arch/arm64/boot/dts/freescale/imx8mm-icore-mx8mm-ctouch2-of10.dtb > DTC arch/arm64/boot/dts/freescale/imx8mm-icore-mx8mm-edimm2.2.dtb > .. > > Can you let me know what I missed here? You pasted here output of validating with property-units.yaml (or something else), not the schema which you changed. If you want to limit the tests, use DT_SCHEMA_FILES. I mentioned about exactly the same problem in yout previous v1 at patch #5. No changes here stil. Best regards, Krzysztof
Re: [PATCH v2 4/6] arm64: dts: imx8mm: Add Engicam i.Core MX8M Mini C.TOUCH 2.0
On Mon, Dec 21, 2020 at 05:01:49PM +0530, Jagan Teki wrote:
> Engicam C.TOUCH 2.0 is an EDIMM compliant general purpose Carrier
> board.
>
> Genaral features:
> - Ethernet 10/100
> - Wifi/BT
> - USB Type A/OTG
> - Audio Out
> - CAN
> - LVDS panel connector
>
> i.Core MX8M Mini is an EDIMM SoM based on NXP i.MX8M Mini from Engicam.
>
> i.Core MX8M Mini needs to mount on top of this Carrier board for
> creating complete i.Core MX8M Mini C.TOUCH 2.0 board.
>
> Add support for it.
>
> Signed-off-by: Matteo Lisi
> Signed-off-by: Jagan Teki
> ---
> Changes for v2:
> - enabled fec1 node
> - updated commit message
> - dropped engicam from filename since it aligned with imx6 engicam
> dts files naming conventions.
> - add i2c nodes
> - fixed v1 comments
>
> arch/arm64/boot/dts/freescale/Makefile| 1 +
> .../dts/freescale/imx8mm-engicam-common.dtsi | 82 +++
> .../dts/freescale/imx8mm-engicam-ctouch2.dtsi | 7 ++
> .../freescale/imx8mm-icore-mx8mm-ctouch2.dts | 21 +
> 4 files changed, 111 insertions(+)
> create mode 100644 arch/arm64/boot/dts/freescale/imx8mm-engicam-common.dtsi
> create mode 100644 arch/arm64/boot/dts/freescale/imx8mm-engicam-ctouch2.dtsi
> create mode 100644
> arch/arm64/boot/dts/freescale/imx8mm-icore-mx8mm-ctouch2.dts
>
> diff --git a/arch/arm64/boot/dts/freescale/Makefile
> b/arch/arm64/boot/dts/freescale/Makefile
> index 6f0777ee6cd6..8d49a2c74604 100644
> --- a/arch/arm64/boot/dts/freescale/Makefile
> +++ b/arch/arm64/boot/dts/freescale/Makefile
> @@ -32,6 +32,7 @@ dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-lx2162a-qds.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mm-beacon-kit.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mm-evk.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mm-ddr4-evk.dtb
> +dtb-$(CONFIG_ARCH_MXC) += imx8mm-icore-mx8mm-ctouch2.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mm-kontron-n801x-s.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mm-var-som-symphony.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mn-evk.dtb
> diff --git a/arch/arm64/boot/dts/freescale/imx8mm-engicam-common.dtsi
> b/arch/arm64/boot/dts/freescale/imx8mm-engicam-common.dtsi
> new file mode 100644
> index ..f7870efd9dab
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx8mm-engicam-common.dtsi
> @@ -0,0 +1,82 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (c) 2020 Engicam srl
> + * Copyright (c) 2020 Amarula Solutions(India)
> + */
> +
> + {
> + status = "okay";
> +};
> +
> + {
> + clock-frequency = <40>;
> + pinctrl-names = "default";
> + pinctrl-0 = <_i2c2>;
> + status = "okay";
> +};
> +
> + {
> + clock-frequency = <10>;
> + pinctrl-names = "default";
> + pinctrl-0 = <_i2c4>;
> + status = "okay";
> +};
> +
> + {
> + pinctrl_i2c2: i2c2grp {
> + fsl,pins = <
> + MX8MM_IOMUXC_I2C2_SCL_I2C2_SCL 0x41c3
> + MX8MM_IOMUXC_I2C2_SDA_I2C2_SDA 0x41c3
> + >;
> + };
> +
> + pinctrl_i2c4: i2c4grp {
> + fsl,pins = <
> + MX8MM_IOMUXC_I2C4_SCL_I2C4_SCL 0x41c3
> + MX8MM_IOMUXC_I2C4_SDA_I2C4_SDA 0x41c3
> + >;
> + };
> +
> + pinctrl_uart2: uart2grp {
> + fsl,pins = <
> + MX8MM_IOMUXC_UART2_RXD_UART2_DCE_RX 0x140
> + MX8MM_IOMUXC_UART2_TXD_UART2_DCE_TX 0x140
> + >;
> + };
> +
> + pinctrl_usdhc1_gpio: usdhc1gpiogrp {
> + fsl,pins = <
> + MX8MM_IOMUXC_GPIO1_IO06_GPIO1_IO6 0x41
> + >;
> + };
> +
> + pinctrl_usdhc1: usdhc1grp {
> + fsl,pins = <
> + MX8MM_IOMUXC_SD1_CLK_USDHC1_CLK 0x190
> + MX8MM_IOMUXC_SD1_CMD_USDHC1_CMD 0x1d0
> + MX8MM_IOMUXC_SD1_DATA0_USDHC1_DATA0 0x1d0
> + MX8MM_IOMUXC_SD1_DATA1_USDHC1_DATA1 0x1d0
> + MX8MM_IOMUXC_SD1_DATA2_USDHC1_DATA2 0x1d0
> + MX8MM_IOMUXC_SD1_DATA3_USDHC1_DATA3 0x1d0
> + >;
> + };
> +};
> +
> + {
> + pinctrl-names = "default";
> + pinctrl-0 = <_uart2>;
> + status = "okay";
> +};
> +
> +/* SD */
> + {
> + pinctrl-names = "default";
> + pinctrl-0 = <_usdhc1>, <_usdhc1_gpio>;
> + cd-gpios = < 6 GPIO_ACTIVE_LOW>;
> + max-frequency = <5000>;
> + bus-width = <4>;
> + no-1-8-v;
> + pm-ignore-notify;
> + keep-power-in-suspend;
> + status = "okay";
> +};
> diff --git a/arch/arm64/boot/dts/freescale/imx8mm-engicam-ctouch2.dtsi
> b/arch/arm64/boot/dts/freescale/imx8mm-engicam-ctouch2.dtsi
> new file mode 100644
> index ..294df07289a2
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx8mm-engicam-ctouch2.dtsi
> @@ -0,0 +1,7 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (c) 2020 Engicam srl
> + * Copyright (c) 2020
Re: [PATCH v2 3/6] arm64: dts: imx8mm: Add Engicam i.Core MX8M Mini SoM
On Mon, Dec 21, 2020 at 05:01:48PM +0530, Jagan Teki wrote:
> i.Core MX8M Mini is an EDIMM SoM based on NXP i.MX8M Mini
> from Engicam.
>
> General features:
> - NXP i.MX8M Mini
> - Up to 2GB LDDR4
> - 8/16GB eMMC
> - Gigabit Ethernet
> - USB 2.0 Host/OTG
> - PCIe Gen2 interface
> - I2S
> - MIPI DSI to LVDS
> - rest of i.MX8M Mini features
>
> i.Core MX8M Mini needs to mount on top of Engicam baseboards
> for creating complete platform solutions.
>
> Add support for it.
>
> Signed-off-by: Matteo Lisi
> Signed-off-by: Jagan Teki
> ---
> Changes for v2:
> - updated commit message
> - add cpu nodes
> - add fec1 node
> - fixed pmic tree comments
> - dropped engicam from filename since it aligned with imx6 engicam
> dts files naming conventions.
Thanks for the changes.
>
> .../dts/freescale/imx8mm-icore-mx8mm.dtsi | 232 ++
> 1 file changed, 232 insertions(+)
> create mode 100644 arch/arm64/boot/dts/freescale/imx8mm-icore-mx8mm.dtsi
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8mm-icore-mx8mm.dtsi
> b/arch/arm64/boot/dts/freescale/imx8mm-icore-mx8mm.dtsi
> new file mode 100644
> index ..e67865fd102a
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx8mm-icore-mx8mm.dtsi
> @@ -0,0 +1,232 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (c) 2018 NXP
> + * Copyright (c) 2019 Engicam srl
> + * Copyright (c) 2020 Amarula Solutons(India)
> + */
> +
> +/ {
> + compatible = "engicam,icore-mx8mm", "fsl,imx8mm";
> +};
> +
> +_0 {
> + cpu-supply = <_buck4>;
> +};
> +
> +_1 {
> + cpu-supply = <_buck4>;
> +};
> +
> +_2 {
> + cpu-supply = <_buck4>;
> +};
> +
> +_3 {
> + cpu-supply = <_buck4>;
> +};
> +
> + {
> + pinctrl-names = "default";
> + pinctrl-0 = <_fec1>;
> + phy-mode = "rgmii-id";
> + phy-handle = <>;
> +
> + mdio {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + ethphy: ethernet-phy@3 {
> + compatible = "ethernet-phy-ieee802.3-c22";
> + reg = <3>;
> + reset-gpios = < 7 GPIO_ACTIVE_LOW>;
> + reset-assert-us = <1>;
> + };
> + };
> +};
> +
> + {
> + clock-frequency = <40>;
> + pinctrl-names = "default";
> + pinctrl-0 = <_i2c1>;
> + status = "okay";
> +
> + pmic@8 {
> + compatible = "nxp,pf8121a";
> + reg = <0x08>;
> +
> + regulators {
> + reg_ldo1: ldo1 {
> + regulator-max-microvolt = <500>;
> + regulator-min-microvolt = <150>;
I mentioned previously min/max hoping it will be obvious (as most or
even all of DTS follow this convention... although not example in your
regulator) but let be more specific: first min, then max. Don't reverse
the logic. See also example in the regulator.yaml.
Best regards,
Krzysztof
Re: [PATCH v2 2/6] dt-bindings: arm: fsl: Add Engicam i.Core MX8M Mini C.TOUCH 2.0
On Mon, Dec 21, 2020 at 05:01:47PM +0530, Jagan Teki wrote: > i.Core MX8M Mini is an EDIMM SoM based on NXP i.MX8M Mini from Engicam. > > C.TOUCH 2.0 is a general purpose carrier board with capacitive > touch interface support. > > i.Core MX8M Mini needs to mount on top of this Carrier board for > creating complete i.Core MX8M Mini C.TOUCH 2.0 board. > > Add bindings for it. > > Signed-off-by: Jagan Teki > --- > Changes for v2: > - updated commit message > > Documentation/devicetree/bindings/arm/fsl.yaml | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml > b/Documentation/devicetree/bindings/arm/fsl.yaml > index 67980dcef66d..e653e0a43016 100644 > --- a/Documentation/devicetree/bindings/arm/fsl.yaml > +++ b/Documentation/devicetree/bindings/arm/fsl.yaml > @@ -667,6 +667,8 @@ properties: > items: >- enum: >- beacon,imx8mm-beacon-kit # i.MX8MM Beacon Development Kit > + - engicam,icore-mx8mm # i.MX8MM Engicam i.Core > MX8M Mini SOM > + - engicam,icore-mx8mm-ctouch2 # i.MX8MM Engicam i.Core > MX8M Mini C.TOUCH 2.0 Please test your DTS against new schema with dtbs_check. This won't match. Submitting bindings and DTS which fail on day 0 is the same as sending code which does not compile. Best regards, Krzysztof >- fsl,imx8mm-ddr4-evk # i.MX8MM DDR4 EVK Board >- fsl,imx8mm-evk# i.MX8MM EVK Board >- kontron,imx8mm-n801x-som # i.MX8MM Kontron SL (N801X) SOM > -- > 2.25.1 >
Re: [PATCH v2 1/6] arm64: defconfig: Enable REGULATOR_PF8X00
On Mon, Dec 21, 2020 at 05:01:46PM +0530, Jagan Teki wrote: > Enable PF8X00 regulator driver by default as it used in > some of i.MX8MM hardware platforms. > > Engicam i.Core MX8M Mini SoM is using the PF8121A family PMIC. > > Cc: Catalin Marinas > Cc: Will Deacon > Signed-off-by: Jagan Teki > --- > Changes for v2: > - updated commit message > > arch/arm64/configs/defconfig | 1 + > 1 file changed, 1 insertion(+) Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH v8 3/4] arm64: dts: imx8m: add NVMEM provider and consumer to read soc unique ID
On Mon, Dec 21, 2020 at 03:10:52AM +, Alice Guo (OSS) wrote: > > > > -Original Message- > > From: Krzysztof Kozlowski > > Sent: 2020年12月19日 20:17 > > To: Alice Guo (OSS) > > Cc: robh...@kernel.org; shawn...@kernel.org; s.ha...@pengutronix.de; > > ker...@pengutronix.de; feste...@gmail.com; devicet...@vger.kernel.org; > > linux-arm-ker...@lists.infradead.org; linux-kernel@vger.kernel.org; > > dl-linux-imx > > > > Subject: Re: [PATCH v8 3/4] arm64: dts: imx8m: add NVMEM provider and > > consumer to read soc unique ID > > > > On Fri, Dec 18, 2020 at 04:37:25PM +0800, Alice Guo (OSS) wrote: > > > From: Alice Guo > > > > > > In order to be able to use NVMEM APIs to read soc unique ID, add the > > > nvmem data cell and name for nvmem-cells to the "soc" node, and add a > > > nvmem node which provides soc unique ID to efuse@3035. > > > > > > Signed-off-by: Alice Guo > > > --- > > > Changes for v8: > > > - none > > > Changes for v7: > > > - add Reviewed-by > > > > What happened with my reviewed-by? > > > > Best regards, > > Krzysztof > > Hi, > I forgot to add reviewed-by. ☹ It was there already, so you had to remove it for some reason... but you kept the changelog. Best regards, Krzysztof
Re: [PATCH 9/9] mfd: sec-irq: Do not enforce (incorrect) interrupt trigger type
On Mon, Dec 21, 2020 at 08:36:02AM +0100, Marek Szyprowski wrote: > Hi Krzysztof, > > On 18.12.2020 15:22, Krzysztof Kozlowski wrote: > > On Fri, Dec 18, 2020 at 02:25:39PM +0100, Marek Szyprowski wrote: > >> On 10.12.2020 22:29, Krzysztof Kozlowski wrote: > >>> Interrupt line can be configured on different hardware in different way, > >>> even inverted. Therefore driver should not enforce specific trigger > >>> type - edge falling - but instead rely on Devicetree to configure it. > >>> > >>> The Samsung PMIC drivers are used only on Devicetree boards. > >>> > >>> Additionally, the PMIC datasheets describe the interrupt line as active > >>> low with a requirement of acknowledge from the CPU therefore the edge > >>> falling is not correct. > >>> > >>> Signed-off-by: Krzysztof Kozlowski > >> Tested-by: Marek Szyprowski > >> > >> It looks that this together with DTS change fixes RTC alarm failure that > >> I've observed from time to time on TM2e board! > > Great! I'll add this to the commit msg. > > > > Thanks for testing. > > BTW, while playing with this, maybe it would make sense to fix the > reported interrupt type for the PMIC sub-interrupts: > > # grep s2mps /proc/interrupts > 120: 0 gpa0 7 Level s2mps13 > 121: 0 s2mps13 10 Edge rtc-alarm0 I also spotted this. It's a virtual interrupt and I am not sure whether we can actually configure it when the hardware does not allow to set the type (the regmap_irq_type requires register offsets). Best regards, Krzysztof
Re: [PATCH v2 3/4] arm64: dts: imx8mq-librem5-devkit: Disable snvs_rtc
On Thu, Dec 17, 2020 at 04:13:14PM +0100, Guido Günther wrote: > The board has it's own RTC chip which is backed by the (optional) > battery and hence preserves data/time on poweroff when that is inserted. > > Signed-off-by: Guido Günther > --- > arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts | 4 > 1 file changed, 4 insertions(+) > Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH v2 2/4] arm64: dts: imx8mq-librem5-devkit: Tweak pmic regulators
On Thu, Dec 17, 2020 at 04:13:13PM +0100, Guido Günther wrote: > BUCK3 needs a regulator-enable-ramp-delay since otherwise the board > freezes on etnaviv probe. With this pgc_gpu suspends and resumes as > expected. This must have been always broken since gpcv2 support was > enabled. > > We also enable all the regulators needed for Deep Sleep Mode (DSM) as > always-on: > > - VDD_SOC supplied by BUCK1 > - VDDA_1P8 supplied by BUCK7 > - VDDA_0P9 supplied by LDO4 > - VDDA_DRAM supplied by LDO3 > - NVCC_DRAM supplied by BUCK8 > - VDD_DRAM supplied by BUCK5 > > Finally LDO5 and LDO6 provide VDD_PHY_1V8 and VDD_PHY_0V9 used by the > SOCs MIPI, HDMI and USB IP cores. While we would in theory be able to > turn these off (and I've tested that or LDO6 and mipi with USB disabled) > it is of little practical use atm since USB doesn't runtime suspend so > let's revisit this at a later point. > > Signed-off-by: Guido Günther > --- > .../boot/dts/freescale/imx8mq-librem5-devkit.dts | 11 +++ > 1 file changed, 11 insertions(+) > Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH v2 4/4] arm64: dts: imx8mq-librem5-devkit: Drop custom clock settings
On Thu, Dec 17, 2020 at 04:13:15PM +0100, Guido Günther wrote: > Otherwise the boot hangs early on and the resulting clock tree without > this already closely matches the selected rates (722534400 and > 786432000). > > audio_pll2 000 722534397 0 > 0 5 > audio_pll2_bypass000 722534397 0 > 0 5 > audio_pll2_out000 722534397 0 > 0 5 > audio_pll1 110 786431998 0 > 0 5 > audio_pll1_bypass110 786431998 0 > 0 5 > audio_pll1_out110 786431998 0 > 0 5 >sai2 11024576000 0 > 0 5 > sai2_root_clk 11024576000 0 >0 5 >sai6 00024576000 0 > 0 5 > sai6_root_clk 00024576000 0 >0 5 > > Signed-off-by: Guido Günther > --- > arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts | 5 - > 1 file changed, 5 deletions(-) > Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
