Re: [PATCH v2 4/4] soc: samsung: exynos-chipid: convert to driver and merge exynos-asv
On Tue, Dec 08, 2020 at 12:31:23PM +0530, Pankaj Dubey wrote: > > > > -Original Message- > > From: Krzysztof Kozlowski > > Sent: Tuesday, December 8, 2020 12:35 AM > > To: Krzysztof Kozlowski ; linux-arm- > > ker...@lists.infradead.org; linux-samsung-...@vger.kernel.org; linux- > > ker...@vger.kernel.org > > Cc: Sylwester Nawrocki ; Marek Szyprowski > > ; Bartlomiej Zolnierkiewicz > > ; Arnd Bergmann ; Chanwoo > > Choi ; Alim Akhtar ; > > Pankaj Dubey > > Subject: [PATCH v2 4/4] soc: samsung: exynos-chipid: convert to driver and > > merge exynos-asv > > > > The Exynos Chip ID driver on Exynos SoCs has so far only informational > > purpose - to expose the SoC device in sysfs. No other drivers depend on > it > > so there is really no benefit of initializing it early. > > > > One of the intention behind initializing Exynos Chip ID driver in early > stage was to simplify code in arch/arm/mach-exynos specifically calls such > as soc_is_exynos. > But there were lot of resistance from community to add any such calls (or > exported function) from mach-exynos files to the driver file. Whereas some > other SoC code is using the same, e.g. tegra_get_chip_id being called from > mach-tegra files to drivers/soc/tegra/. Unfortunately we could not accept > similar solution for Exynos SoC and hence could not get rid of > soc_is_exynosxXXX and similar macros from various file in mach-exynos and > eventually converting those files into a full-fledged driver files. > > Any opinion how can we achieve this if we convert Exynos Chip ID driver to a > regular driver? a. Some parts of mach code can be moved to drivers and then use OF calls. b. The ones which cannot be moved, could use soc_device_match() assuming they are called after the soc-bus is operational - so after core_initcalls. c. The ones which are called early or without cache coherency (soc_device_match() uses krefs()), cannot be converted. This chip ID conversion indeed kills case (b) above... which I am not sure is worth bothering. Which parts of code could be moved like this? Not mentioning that none of this work have happened since few years... Best regards, Krzysztof
Re: [PATCH] arm64: dts: imx8mn: Fix duplicate node name
On Tue, Dec 08, 2020 at 09:03:34AM +0100, Krzysztof Kozlowski wrote:
> On Tue, Dec 08, 2020 at 03:16:35AM +, S.j. Wang wrote:
> > Hi
> >
> > >
> > > On Mon, Dec 07, 2020 at 02:21:40PM +0100, Krzysztof Kozlowski wrote:
> > > > On Mon, Dec 07, 2020 at 02:53:24PM +0800, Shengjiu Wang wrote:
> > > > > Error log:
> > > > > sysfs: cannot create duplicate filename
> > > '/bus/platform/devices/3000.bus'
> > > > >
> > > > > The spba bus name is duplicate with aips bus name.
> > > > > Refine spba bus name to fix this issue.
> > > > >
> > > > > Fixes: 970406eaef3a ("arm64: dts: imx8mn: Enable Asynchronous Sample
> > > > > Rate Converter")
> > > > > Signed-off-by: Shengjiu Wang
> > > > > ---
> > > > > 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 fd669c0f3fe5..30762eb4f0a7 100644
> > > > > --- a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > > > > +++ b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > > > > @@ -246,7 +246,7 @@ aips1: bus@3000 {
> > > > > #size-cells = <1>;
> > > > > ranges;
> > > > >
> > > > > - spba: bus@3000 {
> > > > > + spba: spba-bus@3000 {
> > > >
> > > > The proper node name is "bus" so basically you introduce wrong name to
> > > > other problem. Introducing wrong names at least requires a comment.
> > >
> > > I just noticed that my message was barely understandable... so let me fix
> > > it:
> > >
> > > The proper node name is "bus" so basically you introduce wrong name to
> > > _fix_ other problem. Introducing wrong names at least requires a comment.
> > >
> > > > However the actual problem here is not in node names but in addresses:
> > > >
> > > > aips1: bus@3000 {
> > > > spba: bus@3000 {
> > > >
> > > > You have to devices with the same unit address. How do you share the
> > > > address space?
> > > >
> > > > I think this should be rather fixed.
> > >
> > > And again, hungry keyboard ate a letter, so:
> > >
> > > You have _two_ devices with the same unit address. How do you share the
> > > address space?
> > > I think this should be rather fixed.
> > >
> >
> > spba is the first block of aips1 space, so it has same start address as
> > aips1.
>
> The reference manual describes it "Reserved for SDMA2 internal memory",
> so indeed it is first address but does it have to be mapped?
> Anyway, why don't you use ranges to remove the conflict?
The IO address space remapping could be a solution but there is another
problem - the hardware representation in DT does not match what
reference manual is saying.
The AIPS bus @3000 has several IPs:
- SAI2@3002
- ...
- GPIO1@3020
However in DTS you will find additional SPBA bus for 3000 -
300c. It's not really the SDMA, as SDMA is at different address. It
is rather an address space which SDMA should map... but it is not a bus
with children. Adding spba-bus@3000 with its children does not look
like correct representation of HW in DTS.
Best regards,
Krzysztof
Re: [PATCH 4/4] dt-bindings: hwmon: convert AD ADM1275 bindings to dt-schema
On Mon, Dec 07, 2020 at 04:19:16PM -0800, Guenter Roeck wrote: > On Mon, Dec 07, 2020 at 03:12:59PM -0600, Rob Herring wrote: > > On Tue, Nov 17, 2020 at 11:08:07PM +0100, Krzysztof Kozlowski wrote: > > > Convert the Analog Devices ADM1275 bindings to dt-schema. > > > > > > Signed-off-by: Krzysztof Kozlowski > > > --- > > > .../bindings/hwmon/adi,adm1275.yaml | 58 +++ > > > .../devicetree/bindings/hwmon/adm1275.txt | 25 > > > Documentation/hwmon/adm1275.rst | 2 +- > > > 3 files changed, 59 insertions(+), 26 deletions(-) > > > create mode 100644 > > > Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml > > > delete mode 100644 Documentation/devicetree/bindings/hwmon/adm1275.txt > > > > > > diff --git a/Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml > > > b/Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml > > > new file mode 100644 > > > index ..2cad28c499dc > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml > > > @@ -0,0 +1,58 @@ > > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > > > +%YAML 1.2 > > > +--- > > > + > > > +$id: http://devicetree.org/schemas/hwmon/adi,adm1275.yaml# > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > + > > > +title: Analog Devices ADM1075/ADM127x/ADM129x digital power monitors > > > + > > > +maintainers: > > > + - Krzysztof Kozlowski > > > + > > > +description: | > > > + The ADM1293 and ADM1294 are high accuracy integrated digital power > > > monitors > > > + that offer digital current, voltage, and power monitoring using an > > > on-chip, > > > + 12-bit analog-to-digital converter (ADC), communicated through a PMBus > > > + compliant I2C interface. > > > + > > > + Datasheets: > > > +https://www.analog.com/en/products/adm1294.html > > > + > > > +properties: > > > + compatible: > > > +enum: > > > + - adi,adm1075 > > > + - adi,adm1272 > > > + - adi,adm1275 > > > + - adi,adm1276 > > > + - adi,adm1278 > > > + - adi,adm1293 > > > + - adi,adm1294 > > > + > > > + reg: > > > +maxItems: 1 > > > + > > > + shunt-resistor-micro-ohms: > > > +description: > > > + Shunt resistor value in micro-Ohm. > > > +$ref: /schemas/types.yaml#/definitions/uint32 > > > > Don't need a type if you have units. With that dropped, > > > > Sorry, I am not familiar with the terminology. Does that refer to > the '$ref' line ? Yes, it's the $ref line which can be safely removed. The core dt-schema applies such $ref if the property name ends with '-micro-ohms'. Best regards, Krzysztof
Re: [PATCH 2/2] ARM: dts: aspeed: Add device tree for Ampere's Mt. Jade BMC
On Tue, 8 Dec 2020 at 05:42, Joel Stanley wrote: > > On Tue, 8 Dec 2020 at 04:37, Quan Nguyen wrote: > > > > The Mt. Jade BMC is an ASPEED AST2500-based BMC for the Mt. Jade > > hardware reference platform with Ampere's Altra Processor Family. > > > > Reviewed-by: Andrew Jeffery > > Reviewed-by: Joel Stanley > > Signed-off-by: Quan Nguyen > > Signed-off-by: Phong Vo > > Signed-off-by: Thang Q. Nguyen > > Thanks, I've applied this to the aspeed tree. Did you review it already before (which would explain tags being there)? Best regards, Krzysztof
Re: [PATCH 1/2] dt-bindings: vendor-prefixes: Add an entry for AmpereComputing.com
On Tue, 8 Dec 2020 at 05:41, Joel Stanley wrote: > > Hello Rob, > > On Tue, 8 Dec 2020 at 04:37, Quan Nguyen wrote: > > > > Add "ampere" entry for Ampere Computing LLC: amperecomputing.com > > > > Reviewed-by: Andrew Jeffery > > Reviewed-by: Joel Stanley > > Signed-off-by: Quan Nguyen > > Signed-off-by: Phong Vo > > Signed-off-by: Thang Q. Nguyen It's the first version of the series already with Review tags. Please post them on the list, not off-list. Best regards, Krzysztof
Re: [PATCH] arm64: dts: imx8mn: Fix duplicate node name
On Tue, Dec 08, 2020 at 03:16:35AM +, S.j. Wang wrote:
> Hi
>
> >
> > On Mon, Dec 07, 2020 at 02:21:40PM +0100, Krzysztof Kozlowski wrote:
> > > On Mon, Dec 07, 2020 at 02:53:24PM +0800, Shengjiu Wang wrote:
> > > > Error log:
> > > > sysfs: cannot create duplicate filename
> > '/bus/platform/devices/3000.bus'
> > > >
> > > > The spba bus name is duplicate with aips bus name.
> > > > Refine spba bus name to fix this issue.
> > > >
> > > > Fixes: 970406eaef3a ("arm64: dts: imx8mn: Enable Asynchronous Sample
> > > > Rate Converter")
> > > > Signed-off-by: Shengjiu Wang
> > > > ---
> > > > 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 fd669c0f3fe5..30762eb4f0a7 100644
> > > > --- a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > > > +++ b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > > > @@ -246,7 +246,7 @@ aips1: bus@3000 {
> > > > #size-cells = <1>;
> > > > ranges;
> > > >
> > > > - spba: bus@3000 {
> > > > + spba: spba-bus@3000 {
> > >
> > > The proper node name is "bus" so basically you introduce wrong name to
> > > other problem. Introducing wrong names at least requires a comment.
> >
> > I just noticed that my message was barely understandable... so let me fix
> > it:
> >
> > The proper node name is "bus" so basically you introduce wrong name to
> > _fix_ other problem. Introducing wrong names at least requires a comment.
> >
> > > However the actual problem here is not in node names but in addresses:
> > >
> > > aips1: bus@3000 {
> > > spba: bus@3000 {
> > >
> > > You have to devices with the same unit address. How do you share the
> > > address space?
> > >
> > > I think this should be rather fixed.
> >
> > And again, hungry keyboard ate a letter, so:
> >
> > You have _two_ devices with the same unit address. How do you share the
> > address space?
> > I think this should be rather fixed.
> >
>
> spba is the first block of aips1 space, so it has same start address as
> aips1.
The reference manual describes it "Reserved for SDMA2 internal memory",
so indeed it is first address but does it have to be mapped?
Anyway, why don't you use ranges to remove the conflict?
Best regards,
Krzysztof
[PATCH v2 3/4] soc: samsung: exynos-chipid: order list of SoCs by name
Bring some order to the list of SoCs. No functional change.
Signed-off-by: Krzysztof Kozlowski
---
drivers/soc/samsung/exynos-chipid.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/soc/samsung/exynos-chipid.c
b/drivers/soc/samsung/exynos-chipid.c
index 8d4d05086906..b4cd0cc00f45 100644
--- a/drivers/soc/samsung/exynos-chipid.c
+++ b/drivers/soc/samsung/exynos-chipid.c
@@ -20,6 +20,7 @@ static const struct exynos_soc_id {
const char *name;
unsigned int id;
} soc_ids[] = {
+ /* List ordered by SoC name */
{ "EXYNOS3250", 0xE3472000 },
{ "EXYNOS4210", 0x4320 }, /* EVT0 revision */
{ "EXYNOS4210", 0x4321 },
@@ -29,10 +30,10 @@ static const struct exynos_soc_id {
{ "EXYNOS5260", 0xE526 },
{ "EXYNOS5410", 0xE541 },
{ "EXYNOS5420", 0xE542 },
+ { "EXYNOS5433", 0xE5433000 },
{ "EXYNOS5440", 0xE544 },
{ "EXYNOS5800", 0xE5422000 },
{ "EXYNOS7420", 0xE742 },
- { "EXYNOS5433", 0xE5433000 },
};
static const char * __init product_id_to_soc_id(unsigned int product_id)
--
2.25.1
[PATCH v2 4/4] soc: samsung: exynos-chipid: convert to driver and merge exynos-asv
The Exynos Chip ID driver on Exynos SoCs has so far only informational
purpose - to expose the SoC device in sysfs. No other drivers depend on
it so there is really no benefit of initializing it early.
The code would be the most flexible if converted to a regular driver.
However there is already another driver - Exynos ASV (Adaptive Supply
Voltage) - which binds to the device node of Chip ID.
The solution is to convert the Exynos Chip ID to a built in driver and
merge the Exynos ASV into it.
This has several benefits:
1. Although the Exynos ASV driver binds to a device node present in all
Exynos DTS (generic compatible), it fails to probe except on the
supported ones (only Exynos5422). This means that the regular boot
process has a planned/expected device probe failure.
Merging the ASV into Chip ID will remove this probe failure because
the final driver will always bind, just with disabled ASV features.
2. Allows to use dev_info() as the SoC bus is present (since
core_initcall).
3. Could speed things up because of execution of Chip ID code in a SMP
environment (after bringing up secondary CPUs, unlike early_initcall),
This reduces the amount of work to be done early, when the kernel has
to bring up critical devices.
5. Makes the Chip ID code defer-probe friendly,
Signed-off-by: Krzysztof Kozlowski
---
arch/arm/mach-exynos/Kconfig| 1 -
drivers/soc/samsung/Kconfig | 12 ++---
drivers/soc/samsung/Makefile| 3 +-
drivers/soc/samsung/exynos-asv.c| 45 +
drivers/soc/samsung/exynos-asv.h| 2 +
drivers/soc/samsung/exynos-chipid.c | 75 -
6 files changed, 70 insertions(+), 68 deletions(-)
diff --git a/arch/arm/mach-exynos/Kconfig b/arch/arm/mach-exynos/Kconfig
index 56d272967fc0..5a48abac6af4 100644
--- a/arch/arm/mach-exynos/Kconfig
+++ b/arch/arm/mach-exynos/Kconfig
@@ -13,7 +13,6 @@ menuconfig ARCH_EXYNOS
select ARM_GIC
select EXYNOS_IRQ_COMBINER
select COMMON_CLK_SAMSUNG
- select EXYNOS_ASV
select EXYNOS_CHIPID
select EXYNOS_THERMAL
select EXYNOS_PMU
diff --git a/drivers/soc/samsung/Kconfig b/drivers/soc/samsung/Kconfig
index fc7f48a92288..5745d7e5908e 100644
--- a/drivers/soc/samsung/Kconfig
+++ b/drivers/soc/samsung/Kconfig
@@ -7,21 +7,19 @@ menuconfig SOC_SAMSUNG
if SOC_SAMSUNG
-config EXYNOS_ASV
- bool "Exynos Adaptive Supply Voltage support" if COMPILE_TEST
- depends on (ARCH_EXYNOS && EXYNOS_CHIPID) || COMPILE_TEST
- select EXYNOS_ASV_ARM if ARM && ARCH_EXYNOS
-
# There is no need to enable these drivers for ARMv8
config EXYNOS_ASV_ARM
bool "Exynos ASV ARMv7-specific driver extensions" if COMPILE_TEST
- depends on EXYNOS_ASV
+ depends on EXYNOS_CHIPID
config EXYNOS_CHIPID
- bool "Exynos Chipid controller driver" if COMPILE_TEST
+ bool "Exynos ChipID controller and ASV driver" if COMPILE_TEST
depends on ARCH_EXYNOS || COMPILE_TEST
+ select EXYNOS_ASV_ARM if ARM && ARCH_EXYNOS
select MFD_SYSCON
select SOC_BUS
+ help
+ Support for Samsung Exynos SoC ChipID and Adaptive Supply Voltage.
config EXYNOS_PMU
bool "Exynos PMU controller driver" if COMPILE_TEST
diff --git a/drivers/soc/samsung/Makefile b/drivers/soc/samsung/Makefile
index 59e8e9453f27..0c523a8de4eb 100644
--- a/drivers/soc/samsung/Makefile
+++ b/drivers/soc/samsung/Makefile
@@ -1,9 +1,8 @@
# SPDX-License-Identifier: GPL-2.0
-obj-$(CONFIG_EXYNOS_ASV) += exynos-asv.o
obj-$(CONFIG_EXYNOS_ASV_ARM) += exynos5422-asv.o
-obj-$(CONFIG_EXYNOS_CHIPID)+= exynos-chipid.o
+obj-$(CONFIG_EXYNOS_CHIPID)+= exynos-chipid.o exynos-asv.o
obj-$(CONFIG_EXYNOS_PMU) += exynos-pmu.o
obj-$(CONFIG_EXYNOS_PMU_ARM_DRIVERS) += exynos3250-pmu.o exynos4-pmu.o \
diff --git a/drivers/soc/samsung/exynos-asv.c b/drivers/soc/samsung/exynos-asv.c
index 5daeadc36382..d60af8acc391 100644
--- a/drivers/soc/samsung/exynos-asv.c
+++ b/drivers/soc/samsung/exynos-asv.c
@@ -2,7 +2,9 @@
/*
* Copyright (c) 2019 Samsung Electronics Co., Ltd.
* http://www.samsung.com/
+ * Copyright (c) 2020 Krzysztof Kozlowski
* Author: Sylwester Nawrocki
+ * Author: Krzysztof Kozlowski
*
* Samsung Exynos SoC Adaptive Supply Voltage support
*/
@@ -10,12 +12,7 @@
#include
#include
#include
-#include
-#include
-#include
#include
-#include
-#include
#include
#include
#include
@@ -111,7 +108,7 @@ static int exynos_asv_update_opps(struct exynos_asv *asv)
return 0;
}
-static int exynos_asv_probe(struct platform_device *pdev)
+int exynos_asv_init(struct device *dev, struct regmap *regmap)
{
int (*probe_func)(struct exynos_asv *asv);
struct exynos_asv *asv;
@@ -119,21 +116,16 @@ static int exynos_asv_probe(struct platform_device *pdev)
u32 product_
[PATCH v2 1/4] soc: samsung: exynos-asv: don't defer early on not-supported SoCs
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 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/soc/samsung/exynos-asv.c b/drivers/soc/samsung/exynos-asv.c
index 8abf4dfaa5c5..f653e3533f0f 100644
--- a/drivers/soc/samsung/exynos-asv.c
+++ b/drivers/soc/samsung/exynos-asv.c
@@ -119,11 +119,6 @@ static int exynos_asv_probe(struct platform_device *pdev)
u32 product_id = 0;
int ret, i;
- cpu_dev = get_cpu_device(0);
- ret = dev_pm_opp_get_opp_count(cpu_dev);
- if (ret < 0)
- return -EPROBE_DEFER;
-
asv = devm_kzalloc(>dev, sizeof(*asv), GFP_KERNEL);
if (!asv)
return -ENOMEM;
@@ -144,6 +139,11 @@ static int exynos_asv_probe(struct platform_device *pdev)
return -ENODEV;
}
+ cpu_dev = get_cpu_device(0);
+ ret = dev_pm_opp_get_opp_count(cpu_dev);
+ if (ret < 0)
+ return -EPROBE_DEFER;
+
ret = of_property_read_u32(pdev->dev.of_node, "samsung,asv-bin",
>of_bin);
if (ret < 0)
--
2.25.1
[PATCH v2 2/4] soc: samsung: exynos-asv: handle reading revision register error
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 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/soc/samsung/exynos-asv.c b/drivers/soc/samsung/exynos-asv.c
index f653e3533f0f..5daeadc36382 100644
--- a/drivers/soc/samsung/exynos-asv.c
+++ b/drivers/soc/samsung/exynos-asv.c
@@ -129,7 +129,13 @@ static int exynos_asv_probe(struct platform_device *pdev)
return PTR_ERR(asv->chipid_regmap);
}
- regmap_read(asv->chipid_regmap, EXYNOS_CHIPID_REG_PRO_ID, _id);
+ ret = regmap_read(asv->chipid_regmap, EXYNOS_CHIPID_REG_PRO_ID,
+ _id);
+ if (ret < 0) {
+ dev_err(>dev, "Cannot read revision from ChipID: %d\n",
+ ret);
+ return -ENODEV;
+ }
switch (product_id & EXYNOS_MASK) {
case 0xE5422000:
--
2.25.1
[PATCH v2 0/4] soc: samsung: exynos-chipid and asv improvements
Hi, 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". Best regards, Krzysztof Krzysztof Kozlowski (3): soc: samsung: exynos-asv: handle reading revision register error soc: samsung: exynos-chipid: order list of SoCs by name soc: samsung: exynos-chipid: convert to driver and merge exynos-asv Marek Szyprowski (1): soc: samsung: exynos-asv: don't defer early on not-supported SoCs arch/arm/mach-exynos/Kconfig| 1 - drivers/soc/samsung/Kconfig | 12 ++--- drivers/soc/samsung/Makefile| 3 +- drivers/soc/samsung/exynos-asv.c| 57 - drivers/soc/samsung/exynos-asv.h| 2 + drivers/soc/samsung/exynos-chipid.c | 78 - 6 files changed, 81 insertions(+), 72 deletions(-) -- 2.25.1
[PATCH] soc: fix comment for freeing soc_dev_attr
The soc_dev_attr is stored soc_dev->attr during soc_device_register() so
it could be used till the cleanup call: soc_device_unregister().
Therefore this memory should not be freed prior, but after unregistering
soc device.
Signed-off-by: Krzysztof Kozlowski
---
drivers/base/soc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/base/soc.c b/drivers/base/soc.c
index d34609bb7386..0af5363a582c 100644
--- a/drivers/base/soc.c
+++ b/drivers/base/soc.c
@@ -168,7 +168,7 @@ struct soc_device *soc_device_register(struct
soc_device_attribute *soc_dev_attr
}
EXPORT_SYMBOL_GPL(soc_device_register);
-/* Ensure soc_dev->attr is freed prior to calling soc_device_unregister. */
+/* Ensure soc_dev->attr is freed after calling soc_device_unregister. */
void soc_device_unregister(struct soc_device *soc_dev)
{
device_unregister(_dev->dev);
--
2.25.1
[PATCH] MAINTAINERS: crypto: s5p-sss: drop Kamil Konieczny
E-mails to Kamil Konieczny to his Samsung address bounce with 550 (User unknown). Kamil no longer takes care about Samsung S5P SSS driver so remove the invalid email address from: - mailmap, - bindings maintainer entries, - maintainers entry for S5P Security Subsystem crypto accelerator. Signed-off-by: Krzysztof Kozlowski --- .mailmap | 1 - Documentation/devicetree/bindings/crypto/samsung-slimsss.yaml | 1 - Documentation/devicetree/bindings/crypto/samsung-sss.yaml | 1 - MAINTAINERS | 1 - 4 files changed, 4 deletions(-) diff --git a/.mailmap b/.mailmap index 225546cc8028..ff2699f60a03 100644 --- a/.mailmap +++ b/.mailmap @@ -172,7 +172,6 @@ Juha Yrjola Juha Yrjola Juha Yrjola Julien Thierry -Kamil Konieczny Kay Sievers Kees Cook Kees Cook diff --git a/Documentation/devicetree/bindings/crypto/samsung-slimsss.yaml b/Documentation/devicetree/bindings/crypto/samsung-slimsss.yaml index 7743eae049ab..676950bb7b37 100644 --- a/Documentation/devicetree/bindings/crypto/samsung-slimsss.yaml +++ b/Documentation/devicetree/bindings/crypto/samsung-slimsss.yaml @@ -8,7 +8,6 @@ title: Samsung Exynos SoC SlimSSS (Slim Security SubSystem) module maintainers: - Krzysztof Kozlowski - - Kamil Konieczny description: |+ The SlimSSS module in Exynos5433 SoC supports the following: diff --git a/Documentation/devicetree/bindings/crypto/samsung-sss.yaml b/Documentation/devicetree/bindings/crypto/samsung-sss.yaml index cf1c47a81d7f..6d62b0e42fc9 100644 --- a/Documentation/devicetree/bindings/crypto/samsung-sss.yaml +++ b/Documentation/devicetree/bindings/crypto/samsung-sss.yaml @@ -8,7 +8,6 @@ title: Samsung Exynos SoC SSS (Security SubSystem) module maintainers: - Krzysztof Kozlowski - - Kamil Konieczny description: |+ The SSS module in S5PV210 SoC supports the following: diff --git a/MAINTAINERS b/MAINTAINERS index 12dd1fff2a39..c45d4a3be2e4 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -15620,7 +15620,6 @@ F: drivers/media/i2c/s5k5baf.c SAMSUNG S5P Security SubSystem (SSS) DRIVER M: Krzysztof Kozlowski M: Vladimir Zapolskiy -M: Kamil Konieczny L: linux-cry...@vger.kernel.org L: linux-samsung-...@vger.kernel.org S: Maintained -- 2.25.1
Re: [PATCH net-next] nfc: s3fwrn5: Change irqflags
On Mon, Dec 07, 2020 at 10:39:01PM +0900, Bongsu Jeon wrote: > On Mon, Dec 7, 2020 at 8:51 PM Krzysztof Kozlowski wrote: > > > > On Mon, Dec 07, 2020 at 08:38:27PM +0900, Bongsu Jeon wrote: > > > From: Bongsu Jeon > > > > > > change irqflags from IRQF_TRIGGER_HIGH to IRQF_TRIGGER_RISING for stable > > > Samsung's nfc interrupt handling. > > > > 1. Describe in commit title/subject the change. Just a word "change > > irqflags" is > >not enough. > > > Ok. I'll update it. > > > 2. Describe in commit message what you are trying to fix. Before was not > >stable? The "for stable interrupt handling" is a little bit vauge. > > > Usually, Samsung's NFC Firmware sends an i2c frame as below. > > 1. NFC Firmware sets the gpio(interrupt pin) high when there is an i2c > frame to send. > 2. If the CPU's I2C master has received the i2c frame, NFC F/W sets > the gpio low. > > NFC driver's i2c interrupt handler would be called in the abnormal case > as the NFC F/W task of number 2 is delayed because of other high > priority tasks. > In that case, NFC driver will try to receive the i2c frame but there > isn't any i2c frame > to send in NFC. It would cause an I2C communication problem. > This case would hardly happen. > But, I changed the interrupt as a defense code. > If Driver uses the TRIGGER_RISING not LEVEL trigger, there would be no problem > even if the NFC F/W task is delayed. All this should be explained in commit message, not in the email. > > > 3. This is contradictory to the bindings and current DTS. I think the > >driver should not force the specific trigger type because I could > >imagine some configuration that the actual interrupt to the CPU is > >routed differently. > > > >Instead, how about removing the trigger flags here and fixing the DTS > >and bindings example? > > > > As I mentioned before, > I changed this code because of Samsung NFC's I2C Communication way. > So, I think that it is okay for the nfc driver to force the specific > trigger type( EDGE_RISING). > > What do you think about it? Some different chip or some different hardware implementation could have the signal inverted, e.g. edge falling, not rising. This is rather a theoretical scenario but still such change makes the code more generic, configurable with DTS. Therefore trigger mode should be configured via DTS, not enforced by the driver. Best regards, Krzysztof
Re: [PATCH] arm64: dts: imx8mn: Fix duplicate node name
On Mon, Dec 07, 2020 at 02:21:40PM +0100, Krzysztof Kozlowski wrote:
> On Mon, Dec 07, 2020 at 02:53:24PM +0800, Shengjiu Wang wrote:
> > Error log:
> > sysfs: cannot create duplicate filename '/bus/platform/devices/3000.bus'
> >
> > The spba bus name is duplicate with aips bus name.
> > Refine spba bus name to fix this issue.
> >
> > Fixes: 970406eaef3a ("arm64: dts: imx8mn: Enable Asynchronous Sample Rate
> > Converter")
> > Signed-off-by: Shengjiu Wang
> > ---
> > 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 fd669c0f3fe5..30762eb4f0a7 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > @@ -246,7 +246,7 @@ aips1: bus@3000 {
> > #size-cells = <1>;
> > ranges;
> >
> > - spba: bus@3000 {
> > + spba: spba-bus@3000 {
>
> The proper node name is "bus" so basically you introduce wrong name to
> other problem. Introducing wrong names at least requires a comment.
I just noticed that my message was barely understandable... so let me
fix it:
The proper node name is "bus" so basically you introduce wrong name to
_fix_ other problem. Introducing wrong names at least requires a comment.
> However the actual problem here is not in node names but in addresses:
>
> aips1: bus@3000 {
> spba: bus@3000 {
>
> You have to devices with the same unit address. How do you share the
> address space?
>
> I think this should be rather fixed.
And again, hungry keyboard ate a letter, so:
You have _two_ devices with the same unit address. How do you share the
address space?
I think this should be rather fixed.
Best regards,
Krzysztof
Re: [PATCH] arm64: dts: imx8mn: Fix duplicate node name
On Mon, Dec 07, 2020 at 02:53:24PM +0800, Shengjiu Wang wrote:
> Error log:
> sysfs: cannot create duplicate filename '/bus/platform/devices/3000.bus'
>
> The spba bus name is duplicate with aips bus name.
> Refine spba bus name to fix this issue.
>
> Fixes: 970406eaef3a ("arm64: dts: imx8mn: Enable Asynchronous Sample Rate
> Converter")
> Signed-off-by: Shengjiu Wang
> ---
> 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 fd669c0f3fe5..30762eb4f0a7 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> @@ -246,7 +246,7 @@ aips1: bus@3000 {
> #size-cells = <1>;
> ranges;
>
> - spba: bus@3000 {
> + spba: spba-bus@3000 {
The proper node name is "bus" so basically you introduce wrong name to
other problem. Introducing wrong names at least requires a comment.
However the actual problem here is not in node names but in addresses:
aips1: bus@3000 {
spba: bus@3000 {
You have to devices with the same unit address. How do you share the
address space?
I think this should be rather fixed.
Best regards,
Krzysztof
> compatible = "fsl,spba-bus", "simple-bus";
> #address-cells = <1>;
> #size-cells = <1>;
> --
> 2.27.0
>
Re: [PATCH net-next] nfc: s3fwrn5: Change irqflags
On Mon, Dec 07, 2020 at 08:38:27PM +0900, Bongsu Jeon wrote: > From: Bongsu Jeon > > change irqflags from IRQF_TRIGGER_HIGH to IRQF_TRIGGER_RISING for stable > Samsung's nfc interrupt handling. 1. Describe in commit title/subject the change. Just a word "change irqflags" is not enough. 2. Describe in commit message what you are trying to fix. Before was not stable? The "for stable interrupt handling" is a little bit vauge. 3. This is contradictory to the bindings and current DTS. I think the driver should not force the specific trigger type because I could imagine some configuration that the actual interrupt to the CPU is routed differently. Instead, how about removing the trigger flags here and fixing the DTS and bindings example? Best regards, Krzysztof > > Signed-off-by: Bongsu Jeon > --- > drivers/nfc/s3fwrn5/i2c.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/nfc/s3fwrn5/i2c.c b/drivers/nfc/s3fwrn5/i2c.c > index e1bdde105f24..016f6b6df849 100644 > --- a/drivers/nfc/s3fwrn5/i2c.c > +++ b/drivers/nfc/s3fwrn5/i2c.c > @@ -213,7 +213,7 @@ static int s3fwrn5_i2c_probe(struct i2c_client *client, > return ret; > > ret = devm_request_threaded_irq(>dev, phy->i2c_dev->irq, NULL, > - s3fwrn5_i2c_irq_thread_fn, IRQF_TRIGGER_HIGH | IRQF_ONESHOT, > + s3fwrn5_i2c_irq_thread_fn, IRQF_TRIGGER_RISING | IRQF_ONESHOT, > S3FWRN5_I2C_DRIVER_NAME, phy); > if (ret) > s3fwrn5_remove(phy->common.ndev); > -- > 2.17.1 >
Re: [PATCH v10 17/19] ARM: tegra: Add EMC OPP properties to Tegra20 device-trees
On Fri, Dec 04, 2020 at 04:54:55PM +0100, Thierry Reding wrote: > On Tue, Dec 01, 2020 at 01:57:44AM +0300, Dmitry Osipenko wrote: > > 01.12.2020 00:17, Jon Hunter пишет: > > > Hi Dmitry, > > > > > > On 23/11/2020 00:27, Dmitry Osipenko wrote: > > >> Add EMC OPP DVFS tables and update board device-trees by removing > > >> unsupported OPPs. > > >> > > >> Signed-off-by: Dmitry Osipenko > > > This change is generating the following warning on Tegra20 Ventana > > > and prevents the EMC from probing ... > > > > > > [2.485711] tegra20-emc 7000f400.memory-controller: device-tree > > > doesn't have memory timings > > > [2.499386] tegra20-emc 7000f400.memory-controller: 32bit DRAM bus > > > [2.505810] [ cut here ] > > > [2.510511] WARNING: CPU: 0 PID: 1 at > > > /local/workdir/tegra/mlt-linux_next/kernel/drivers/opp/of.c:875 > > > _of_add_opp_table_v2+0x598/0x61c > > > [2.529746] Modules linked in: > > > [2.540140] CPU: 0 PID: 1 Comm: swapper/0 Not tainted > > > 5.10.0-rc5-next-20201130 #1 > > > [2.554606] Hardware name: NVIDIA Tegra SoC (Flattened Device Tree) > > > [2.560892] [] (unwind_backtrace) from [] > > > (show_stack+0x10/0x14) > > > [2.568640] [] (show_stack) from [] > > > (dump_stack+0xc8/0xdc) > > > [2.575866] [] (dump_stack) from [] > > > (__warn+0x104/0x108) > > > [2.582912] [] (__warn) from [] > > > (warn_slowpath_fmt+0xb0/0xb8) > > > [2.590397] [] (warn_slowpath_fmt) from [] > > > (_of_add_opp_table_v2+0x598/0x61c) > > > [2.599269] [] (_of_add_opp_table_v2) from [] > > > (dev_pm_opp_of_add_table+0x3c/0x1a0) > > > [2.608582] [] (dev_pm_opp_of_add_table) from [] > > > (tegra_emc_probe+0x478/0x940) > > > [2.617548] [] (tegra_emc_probe) from [] > > > (platform_drv_probe+0x48/0x98) > > > [2.625899] [] (platform_drv_probe) from [] > > > (really_probe+0x218/0x3b8) > > > [2.634162] [] (really_probe) from [] > > > (driver_probe_device+0x5c/0xb4) > > > [2.642338] [] (driver_probe_device) from [] > > > (device_driver_attach+0x58/0x60) > > > [2.651208] [] (device_driver_attach) from [] > > > (__driver_attach+0x80/0xbc) > > > [2.659730] [] (__driver_attach) from [] > > > (bus_for_each_dev+0x74/0xb4) > > > [2.667905] [] (bus_for_each_dev) from [] > > > (bus_add_driver+0x164/0x1e8) > > > [2.676168] [] (bus_add_driver) from [] > > > (driver_register+0x7c/0x114) > > > [2.684259] [] (driver_register) from [] > > > (do_one_initcall+0x54/0x2b0) > > > [2.692441] [] (do_one_initcall) from [] > > > (kernel_init_freeable+0x1a4/0x1f4) > > > [2.701145] [] (kernel_init_freeable) from [] > > > (kernel_init+0x8/0x118) > > > [2.709321] [] (kernel_init) from [] > > > (ret_from_fork+0x14/0x24) > > > [2.716885] Exception stack(0xc1501fb0 to 0xc1501ff8) > > > [2.721933] 1fa0: > > > > > > [2.730106] 1fc0: > > > > > > [2.738278] 1fe0: 0013 > > > [2.751940] ---[ end trace 61e3b76deca27ef3 ]--- > > > > > > > > > Cheers > > > Jon > > > > > > > Hello Jon, > > > > That is harmless and expected to happen because the patch "memory: > > tegra20: Support hardware versioning and clean up OPP table > > initialization" isn't applied yet, while Thierry already applied the DT > > patches from this v10. > > Hmm... that's new. Since when are device tree additions expected to > cause these kinds of splats? It looks rather as inaccurate message, but except the message itself, no functionality was lost. > Anyway, I did apply these because I had seen at least some of the memory > controller driver patches appear in linux-next and hence had assumed > that the whole series had gone in, not realizing there was anything left > to do. > > Krzysztof, what's your schedule for the memory controller tree? My > recollection is that this will feed into ARM SoC, so if the -rc6 dead- > line applies like it does for platforms, then I may need to revert the > DT patch that causes this so that we don't have to drag this along > through all of the release cycle. If there's still time for you to send > that PR, perhaps we can get the remainder of the Tegra interconnect > series merged for v5.11 as well? I was waiting for last acks from Rob and you and actually planned to merge everything this week (weekend at the latest). Indeed it slightly slipped away... the v11 was reposted late. It could still make till v5.11, if I send the PR now (still around 3 weeks before merge window). However I saw now your comments for the patch 4/10 from v11. I'll take patches 1-3 for now. Best regards, Krzysztof
Re: [PATCH 1/2] soc: samsung: exynos-chipid: order list of SoCs by name
On Wed, 2 Dec 2020 21:59:54 +0200, Krzysztof Kozlowski wrote: > Bring some order to the list of SoCs. No functional change. Applied, thanks! [1/2] soc: samsung: exynos-chipid: order list of SoCs by name [2/2] soc: samsung: exynos-chipid: initialize later - with arch_initcall commit: 3b4c362e5ef102ca2d70d33f4e8cf0780053a7db Best regards, -- Krzysztof Kozlowski
Re: [PATCH] memory: jz4780_nemc: Fix potential NULL dereference in jz4780_nemc_probe()
On Fri, 4 Dec 2020 16:31:57 +0800, Zhang Changzhong wrote: > platform_get_resource() may fail and return NULL, so we should > better check it's return value to avoid a NULL pointer dereference > a bit later in the code. > > This is detected by Coccinelle semantic patch. > > @@ > expression pdev, res, n, t, e, e1, e2; > @@ > > [...] Applied, thanks! [1/1] memory: jz4780_nemc: Fix potential NULL dereference in jz4780_nemc_probe() commit: 4bfa07300b9334b487ed4f3d4901c35ebb31b7ca Best regards, -- Krzysztof Kozlowski
Re: [PATCH] [v2] clk: samsung: mark PM functions as __maybe_unused
On Fri, 4 Dec 2020 10:16:11 +0100, Arnd Bergmann wrote: > The use of SIMPLE_DEV_PM_OPS() means that the suspend/resume > functions are now unused when CONFIG_PM is disabled: > > drivers/clk/samsung/clk-exynos-clkout.c:219:12: error: 'exynos_clkout_resume' > defined but not used [-Werror=unused-function] > 219 | static int exynos_clkout_resume(struct device *dev) > |^~~~ > drivers/clk/samsung/clk-exynos-clkout.c:210:12: error: > 'exynos_clkout_suspend' defined but not used [-Werror=unused-function] > 210 | static int exynos_clkout_suspend(struct device *dev) > |^ > > [...] Applied, thanks! [1/1] clk: samsung: mark PM functions as __maybe_unused commit: 4c44274ee457e3f7012dc532c8c9cc8964a82612 Best regards, -- Krzysztof Kozlowski
Re: [PATCH] memory: ti-emif-sram: only build for ARMv7
On Fri, 4 Dec 2020 00:08:14 +0100, Arnd Bergmann wrote: > The driver can be compile-tested on all ARM machines, but > causes a failure when built for ARMv7-M: > > arm-linux-gnueabi-ld: error: drivers/memory/ti-emif-sram-pm.o: conflicting > architecture profiles A/M > > Limit the target machines to configurations that have ARMv7 enabled. Applied, thanks! [1/1] memory: ti-emif-sram: only build for ARMv7 commit: d77d22d701b0471584abe1871570bb43deb6e3c4 Best regards, -- Krzysztof Kozlowski
Re: [PATCH v11 00/10] Introduce memory interconnect for NVIDIA Tegra SoCs
On Thu, 3 Dec 2020 22:24:29 +0300, Dmitry Osipenko wrote: > This series brings initial support for memory interconnect to Tegra20, > Tegra30 and Tegra124 SoCs. > > For the starter only display controllers and devfreq devices are getting > interconnect API support, others could be supported later on. The display > controllers have the biggest demand for interconnect API right now because > dynamic memory frequency scaling can't be done safely without taking into > account bandwidth requirement from the displays. In particular this series > fixes distorted display output on T30 Ouya and T124 TK1 devices. > > [...] Applied, thanks! [01/10] dt-bindings: memory: tegra20: emc: Document opp-supported-hw property [02/10] memory: tegra20: Support hardware versioning and clean up OPP table initialization [03/10] memory: tegra30: Support interconnect framework commit: 01a51facb74fb337ff9fe734caa85dd6e246ef48 Best regards, -- Krzysztof Kozlowski
Re: [PATCH] [v2] clk: samsung: mark PM functions as __maybe_unused
On Fri, 4 Dec 2020 at 16:28, Sylwester Nawrocki wrote:
>
> From: Arnd Bergmann
>
> The use of SIMPLE_DEV_PM_OPS() means that the suspend/resume
> functions are now unused when CONFIG_PM is disabled:
>
> drivers/clk/samsung/clk-exynos-clkout.c:219:12: error:
> 'exynos_clkout_resume' defined but not used [-Werror=unused-function]
>219 | static int exynos_clkout_resume(struct device *dev)
>|^~~~
> drivers/clk/samsung/clk-exynos-clkout.c:210:12: error:
> 'exynos_clkout_suspend' defined but not used [-Werror=unused-function]
>210 | static int exynos_clkout_suspend(struct device *dev)
>|^
>
> Mark them as __maybe_unused to shut up the otherwise harmless warning.
>
> Fixes: 9484f2cb8332 ("clk: samsung: exynos-clkout: convert to module
> driver")
> Reviewed-by: Krzysztof Kozlowski
> Signed-off-by: Arnd Bergmann
> ---
> v2: add proper changelog text
>
> Acked-by: Sylwester Nawrocki
Indeed this should go via samsung soc tree...
Best regards,
Krzysztof
Re: [PATCH v2 net-next] nfc: s3fwrn5: skip the NFC bootloader mode
On Fri, Dec 04, 2020 at 07:52:57AM +0900, Bongsu Jeon wrote: > From: Bongsu Jeon > > If there isn't a proper NFC firmware image, Bootloader mode will be > skipped. > > Signed-off-by: Bongsu Jeon > --- > > ChangeLog: > v2: >- change the commit message. >- change the skip handling code. Patch is now much cleaner and smaller. Thanks. Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof > > drivers/nfc/s3fwrn5/core.c | 23 +-- > drivers/nfc/s3fwrn5/firmware.c | 11 +-- > drivers/nfc/s3fwrn5/firmware.h | 1 + > 3 files changed, 23 insertions(+), 12 deletions(-)
Re: [PATCH] clk: samsung: mark PM functions as __maybe_unused
On Thu, Dec 03, 2020 at 11:53:11PM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann
>
I understand this happens with !PM builds. It would be good to mention
this in commit msg. With commit msg improved:
Reviewed-by: Krzysztof Kozlowski
Best regards,
Krzysztof
> drivers/clk/samsung/clk-exynos-clkout.c:219:12: error: 'exynos_clkout_resume'
> defined but not used [-Werror=unused-function]
> 219 | static int exynos_clkout_resume(struct device *dev)
> |^~~~
> drivers/clk/samsung/clk-exynos-clkout.c:210:12: error:
> 'exynos_clkout_suspend' defined but not used [-Werror=unused-function]
> 210 | static int exynos_clkout_suspend(struct device *dev)
> |^
>
> Signed-off-by: Arnd Bergmann
> ---
> drivers/clk/samsung/clk-exynos-clkout.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clk/samsung/clk-exynos-clkout.c
> b/drivers/clk/samsung/clk-exynos-clkout.c
> index 9ec2f40cc400..e6d6cbf8c4e6 100644
> --- a/drivers/clk/samsung/clk-exynos-clkout.c
> +++ b/drivers/clk/samsung/clk-exynos-clkout.c
> @@ -207,7 +207,7 @@ static int exynos_clkout_remove(struct platform_device
> *pdev)
> return 0;
> }
>
> -static int exynos_clkout_suspend(struct device *dev)
> +static int __maybe_unused exynos_clkout_suspend(struct device *dev)
> {
> struct exynos_clkout *clkout = dev_get_drvdata(dev);
>
> @@ -216,7 +216,7 @@ static int exynos_clkout_suspend(struct device *dev)
> return 0;
> }
>
> -static int exynos_clkout_resume(struct device *dev)
> +static int __maybe_unused exynos_clkout_resume(struct device *dev)
> {
> struct exynos_clkout *clkout = dev_get_drvdata(dev);
>
> --
> 2.27.0
>
Re: [PATCH 2/2] drm/ingenic: depend on COMMON_CLK to fix compile tests
On Mon, Nov 16, 2020 at 07:54:03PM +, Paul Cercueil wrote: > Hi Krzysztof, > > Le lun. 16 nov. 2020 à 18:53, Krzysztof Kozlowski a écrit > : > > The Ingenic DRM uses Common Clock Framework thus it cannot be built on > > platforms without it (e.g. compile test on MIPS with RALINK and > > SOC_RT305X): > > > > /usr/bin/mips-linux-gnu-ld: > > drivers/gpu/drm/ingenic/ingenic-drm-drv.o: in function > > `ingenic_drm_bind.isra.0': > > ingenic-drm-drv.c:(.text+0x1600): undefined reference to > > `clk_get_parent' > > /usr/bin/mips-linux-gnu-ld: ingenic-drm-drv.c:(.text+0x16b0): > > undefined reference to `clk_get_parent' > > > > Reported-by: kernel test robot > > Signed-off-by: Krzysztof Kozlowski > > Acked-by: Paul Cercueil Thanks for the ack. David and Daniel, I think there is no dedicated maintainer for Ingenic DRM, so can you pick it up directly? Best regards, Krzysztof
Re: [PATCH net-next] nfc: s3fwrn5: skip the NFC bootloader mode
On Fri, Dec 04, 2020 at 12:39:50AM +0900, Bongsu Jeon wrote:
> From: Bongsu Jeon
>
> If there isn't proper NFC firmware image,
> Bootloader mode will be skipped.
Wrap your commit msg as described in submitting patches (so at 75
character).
>
> Signed-off-by: Bongsu Jeon
> ---
> drivers/nfc/s3fwrn5/core.c | 44 --
> drivers/nfc/s3fwrn5/firmware.c | 11 +
> drivers/nfc/s3fwrn5/firmware.h | 1 +
> 3 files changed, 33 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/nfc/s3fwrn5/core.c b/drivers/nfc/s3fwrn5/core.c
> index f8e5d78d9078..df89bc5d7338 100644
> --- a/drivers/nfc/s3fwrn5/core.c
> +++ b/drivers/nfc/s3fwrn5/core.c
> @@ -20,13 +20,26 @@
> NFC_PROTO_ISO14443_B_MASK | \
> NFC_PROTO_ISO15693_MASK)
>
> +static int s3fwrn5_firmware_init(struct s3fwrn5_info *info)
> +{
> + struct s3fwrn5_fw_info *fw_info = >fw_info;
> + int ret;
> +
> + s3fwrn5_fw_init(fw_info, "sec_s3fwrn5_firmware.bin");
> +
> + /* Get firmware data */
> + ret = s3fwrn5_fw_request_firmware(fw_info);
> + if (ret < 0)
> + dev_err(_info->ndev->nfc_dev->dev,
> + "Failed to get fw file, ret=%02x\n", ret);
> + return ret;
> +}
> +
> static int s3fwrn5_firmware_update(struct s3fwrn5_info *info)
> {
> bool need_update;
> int ret;
>
> - s3fwrn5_fw_init(>fw_info, "sec_s3fwrn5_firmware.bin");
> -
> /* Update firmware */
>
> s3fwrn5_set_wake(info, false);
> @@ -109,21 +122,26 @@ static int s3fwrn5_nci_post_setup(struct nci_dev *ndev)
> struct s3fwrn5_info *info = nci_get_drvdata(ndev);
> int ret;
>
> - ret = s3fwrn5_firmware_update(info);
> - if (ret < 0)
> - goto out;
> + if (s3fwrn5_firmware_init(info) == 0) {
if (s3fwrn5_firmware_init(info)) {
// skip bootloader mode
ret = 0;
goto out;
}
so entire next block won't have to be indented. This follows usual
pattern of error handling.
Best regards,
Krzysztof
> + ret = s3fwrn5_firmware_update(info);
> + if (ret < 0)
> + goto out;
>
> - /* NCI core reset */
> -
> - s3fwrn5_set_mode(info, S3FWRN5_MODE_NCI);
> - s3fwrn5_set_wake(info, true);
> + /* NCI core reset */
>
> - ret = nci_core_reset(info->ndev);
> - if (ret < 0)
> - goto out;
> + s3fwrn5_set_mode(info, S3FWRN5_MODE_NCI);
> + s3fwrn5_set_wake(info, true);
>
> - ret = nci_core_init(info->ndev);
> + ret = nci_core_reset(info->ndev);
> + if (ret < 0)
> + goto out;
>
> + ret = nci_core_init(info->ndev);
> + } else {
> + dev_info(>ndev->nfc_dev->dev,
> + "skip bootloader mode\n");
> + ret = 0;
> + }
> out:
> return ret;
> }
Re: [PATCH v2 4/5] amba: Make the remove callback return void
On Thu, Dec 03, 2020 at 02:01:41PM +0100, Uwe Kleine-König wrote: > Hello, > > [This is a resend because somehow my MUA failed to parse the To: list > and dropped it without me noticing it. Sorry to those who got it twice > now.] > > On Tue, Nov 24, 2020 at 02:31:38PM +0100, Uwe Kleine-König wrote: > > From: Uwe Kleine-König > > > > All amba drivers return 0 in their remove callback. Together with the > > driver core ignoring the return value anyhow, it doesn't make sense to > > return a value here. > > > > Change the remove prototype to return void, which makes it explicit that > > returning an error value doesn't work as expected. This simplifies changing > > the core remove callback to return void, too. > > > > Reviewed-by: Ulf Hansson > > Reviewed-by: Arnd Bergmann > > Signed-off-by: Uwe Kleine-König > > --- > > drivers/amba/bus.c | 5 ++--- > > drivers/char/hw_random/nomadik-rng.c | 3 +-- > > drivers/dma/pl330.c| 3 +-- > > drivers/gpu/drm/pl111/pl111_drv.c | 4 +--- > > drivers/hwtracing/coresight/coresight-catu.c | 3 +-- > > drivers/hwtracing/coresight/coresight-cpu-debug.c | 4 +--- > > drivers/hwtracing/coresight/coresight-cti-core.c | 4 +--- > > drivers/hwtracing/coresight/coresight-etb10.c | 4 +--- > > drivers/hwtracing/coresight/coresight-etm3x-core.c | 4 +--- > > drivers/hwtracing/coresight/coresight-etm4x-core.c | 4 +--- > > drivers/hwtracing/coresight/coresight-funnel.c | 4 ++-- > > drivers/hwtracing/coresight/coresight-replicator.c | 4 ++-- > > drivers/hwtracing/coresight/coresight-stm.c| 4 +--- > > drivers/hwtracing/coresight/coresight-tmc-core.c | 4 +--- > > drivers/hwtracing/coresight/coresight-tpiu.c | 4 +--- > > drivers/i2c/busses/i2c-nomadik.c | 4 +--- > > drivers/input/serio/ambakmi.c | 3 +-- > > drivers/memory/pl172.c | 4 +--- > > drivers/memory/pl353-smc.c | 4 +--- For the memory: Acked-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH v2 0/5] memory: renesas-rpc-if: Trivial fixes
On Thu, Dec 03, 2020 at 10:41:54AM +, Lad, Prabhakar wrote:
> Hi Krzysztof,
>
> On Thu, Nov 26, 2020 at 7:11 PM Lad Prabhakar
> wrote:
> >
> > Hi All,
> >
> > This patch series fixes trivial issues in RPC-IF driver.
> >
> > Changes for v2:
> > * Balanced PM in rpcif_disable_rpm
> > * Fixed typo in patch 4/5
> > * Dropped C++ style fixes patch
> > * Included RB tags from Sergei
> >
> > Cheers,
> > Prabhakar
> >
> > Lad Prabhakar (5):
> > memory: renesas-rpc-if: Return correct value to the caller of
> > rpcif_manual_xfer()
> > memory: renesas-rpc-if: Fix unbalanced pm_runtime_enable in
> > rpcif_{enable,disable}_rpm
> > memory: renesas-rpc-if: Fix a reference leak in rpcif_probe()
> > memory: renesas-rpc-if: Make rpcif_enable/disable_rpm() as static
> > inline
> > memory: renesas-rpc-if: Export symbols as GPL
> >
> As these are fixes to the existing driver will these be part of v5.10 release
> ?
Quick look with:
git lg v5.9..v5.10-rc1 -- drivers/memory/
did not show that this driver was added in v5.10-rc1, so the fixes are
not planned to be for v5.10 release never. Why they should be? Maybe I
missed something here?
Best regards,
Krzysztof
Re: [PATCH 3/3] ARM: dts: exynos: Fix charging regulator voltage and current for i9100
On Thu, Dec 03, 2020 at 10:23:01AM +0200, Krzysztof Kozlowski wrote:
> On Thu, Dec 03, 2020 at 05:46:03AM +, Timon Bätz wrote:
> > On Wednesday, December 2, 2020 11:04 PM, Krzysztof Kozlowski
> > wrote:
> >
> > > On Wed, Dec 02, 2020 at 09:07:28PM +, Timon Baetz wrote:
> > >
> > > > Set CHARGER current and CHARGER_CV voltage according to Galaxy S2 kernel
> > > > fork.
> > > >
> > > > Signed-off-by: Timon Baetz timon.ba...@protonmail.com
> > > >
> > > > --
> > > >
> > > > arch/arm/boot/dts/exynos4210-i9100.dts | 8
> > > > 1 file changed, 4 insertions(+), 4 deletions(-)
> > > > diff --git a/arch/arm/boot/dts/exynos4210-i9100.dts
> > > > b/arch/arm/boot/dts/exynos4210-i9100.dts
> > > > index 9f8d927e0d21..2700d53ea01b 100644
> > > > --- a/arch/arm/boot/dts/exynos4210-i9100.dts
> > > > +++ b/arch/arm/boot/dts/exynos4210-i9100.dts
> > > > @@ -558,14 +558,14 @@ safe2_sreg: ESAFEOUT2 {
> > > >
> > > > charger_reg: CHARGER {
> > > > regulator-name = "CHARGER";
> > > >
> > > >
> > > > - regulator-min-microamp = <6>;
> > > >
> > > >
> > > > - regulator-max-microamp = <258>;
> > > >
> > > >
> > > >
> > > > - regulator-min-microamp = <20>;
> > > >
> > > >
> > > > - regulator-max-microamp = <95>;
> > > > };
> > > >
> > > > chargercv_reg: CHARGER_CV {
> > > > regulator-name = "CHARGER_CV";
> > > >
> > > >
> > > >
> > > > - regulator-min-microvolt = <380>;
> > > >
> > > >
> > > > - regulator-max-microvolt = <410>;
> > > >
> > > >
> > > >
> > > > - regulator-min-microvolt = <420>;
> > > >
> > > >
> > > > - regulator-max-microvolt = <420>;
> > > >
> > > >
> > >
> > > I am looking at my sources of Android 3.0 for GT-I9100 but I cannot find
> > > charger voltages for it. Where did you find it?
> > >
> > > Best regards,
> > > Krzysztof
> >
> > Thanks all the feedback Krzysztof,
> >
> > Voltage is set in the charger probe function of the downstream kernel fork:
> > https://github.com/LineageOS/android_kernel_samsung_smdk4412/blob/lineage-17.0/drivers/power/max8997_charger_u1.c#L390-L391
>
> You need to fix your email client to wrap lines.
>
> The fork cannot be used as a reference because of poor quality of
> explanations for origins of the code.
>
> The commit which added 4.2 V is described as "samsung update 1" which
> basically means nothing. If at least it was "drop sources of
> GT-I9105"... but in this form it is useless.
>
> For the things we are not sure how they should be implemented, we
> sometimes accept the reason "vendor sources do like this". However Lineage
> or any other fork are not vendor sources.
>
> Therefore you need to provide a valid explanation for this voltage
> change.
I checked vendor sources for Samsung Galaxy S2 Epic 4G Touch (SPH-D710)
and indeed it uses the max8997 charger U1 which sets v4.2 volts.
You can use it to fix up the commit msg.
Unfortunately it seems Samsung started to remove most of older
kernel source code from their OS compliance page. S1, S2 and S3 are
mostly gone. I was able to find just few remaining sources and I am now
updating my vendor-dump with them. I'll upload them later to
https://github.com/krzk/linux-vendor-backup .
Best regards,
Krzysztof
Re: [PATCH 3/3] ARM: dts: exynos: Fix charging regulator voltage and current for i9100
On Thu, Dec 03, 2020 at 05:46:03AM +, Timon Bätz wrote:
> On Wednesday, December 2, 2020 11:04 PM, Krzysztof Kozlowski
> wrote:
>
> > On Wed, Dec 02, 2020 at 09:07:28PM +, Timon Baetz wrote:
> >
> > > Set CHARGER current and CHARGER_CV voltage according to Galaxy S2 kernel
> > > fork.
> > >
> > > Signed-off-by: Timon Baetz timon.ba...@protonmail.com
> > >
> > > --
> > >
> > > arch/arm/boot/dts/exynos4210-i9100.dts | 8
> > > 1 file changed, 4 insertions(+), 4 deletions(-)
> > > diff --git a/arch/arm/boot/dts/exynos4210-i9100.dts
> > > b/arch/arm/boot/dts/exynos4210-i9100.dts
> > > index 9f8d927e0d21..2700d53ea01b 100644
> > > --- a/arch/arm/boot/dts/exynos4210-i9100.dts
> > > +++ b/arch/arm/boot/dts/exynos4210-i9100.dts
> > > @@ -558,14 +558,14 @@ safe2_sreg: ESAFEOUT2 {
> > >
> > > charger_reg: CHARGER {
> > > regulator-name = "CHARGER";
> > >
> > >
> > > - regulator-min-microamp = <6>;
> > >
> > >
> > > - regulator-max-microamp = <258>;
> > >
> > >
> > >
> > > - regulator-min-microamp = <20>;
> > >
> > >
> > > - regulator-max-microamp = <95>;
> > > };
> > >
> > > chargercv_reg: CHARGER_CV {
> > > regulator-name = "CHARGER_CV";
> > >
> > >
> > >
> > > - regulator-min-microvolt = <380>;
> > >
> > >
> > > - regulator-max-microvolt = <410>;
> > >
> > >
> > >
> > > - regulator-min-microvolt = <420>;
> > >
> > >
> > > - regulator-max-microvolt = <420>;
> > >
> > >
> >
> > I am looking at my sources of Android 3.0 for GT-I9100 but I cannot find
> > charger voltages for it. Where did you find it?
> >
> > Best regards,
> > Krzysztof
>
> Thanks all the feedback Krzysztof,
>
> Voltage is set in the charger probe function of the downstream kernel fork:
> https://github.com/LineageOS/android_kernel_samsung_smdk4412/blob/lineage-17.0/drivers/power/max8997_charger_u1.c#L390-L391
You need to fix your email client to wrap lines.
The fork cannot be used as a reference because of poor quality of
explanations for origins of the code.
The commit which added 4.2 V is described as "samsung update 1" which
basically means nothing. If at least it was "drop sources of
GT-I9105"... but in this form it is useless.
For the things we are not sure how they should be implemented, we
sometimes accept the reason "vendor sources do like this". However Lineage
or any other fork are not vendor sources.
Therefore you need to provide a valid explanation for this voltage
change.
Best regards,
Krzysztof
Re: [PATCH 3/3] ARM: dts: exynos: Fix charging regulator voltage and current for i9100
On Wed, Dec 02, 2020 at 09:07:28PM +, Timon Baetz wrote:
> Set CHARGER current and CHARGER_CV voltage according to Galaxy S2 kernel
> fork.
>
> Signed-off-by: Timon Baetz
> ---
> arch/arm/boot/dts/exynos4210-i9100.dts | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/boot/dts/exynos4210-i9100.dts
> b/arch/arm/boot/dts/exynos4210-i9100.dts
> index 9f8d927e0d21..2700d53ea01b 100644
> --- a/arch/arm/boot/dts/exynos4210-i9100.dts
> +++ b/arch/arm/boot/dts/exynos4210-i9100.dts
> @@ -558,14 +558,14 @@ safe2_sreg: ESAFEOUT2 {
>
> charger_reg: CHARGER {
> regulator-name = "CHARGER";
> - regulator-min-microamp = <6>;
> - regulator-max-microamp = <258>;
> + regulator-min-microamp = <20>;
> + regulator-max-microamp = <95>;
> };
>
> chargercv_reg: CHARGER_CV {
> regulator-name = "CHARGER_CV";
> - regulator-min-microvolt = <380>;
> - regulator-max-microvolt = <410>;
> + regulator-min-microvolt = <420>;
> + regulator-max-microvolt = <420>;
I am looking at my sources of Android 3.0 for GT-I9100 but I cannot find
charger voltages for it. Where did you find it?
Best regards,
Krzysztof
Re: [PATCH 2/3] power: supply: max8997_charger: Set CHARGER current limit
On Wed, Dec 02, 2020 at 09:07:19PM +, Timon Baetz wrote:
> Register for extcon notification and set charging current depending on
> the detected cable type. Current values are taken from i9100 kernel
> fork.
>
> Enable and disable the CHARGER regulator based on extcon events and
> remove regulator-always-on from the device tree.
>
> Signed-off-by: Timon Baetz
> ---
> arch/arm/boot/dts/exynos4210-i9100.dts | 1 -
> drivers/power/supply/max8997_charger.c | 92 ++
> 2 files changed, 92 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/exynos4210-i9100.dts
> b/arch/arm/boot/dts/exynos4210-i9100.dts
> index 6d0c04d77a39..9f8d927e0d21 100644
> --- a/arch/arm/boot/dts/exynos4210-i9100.dts
> +++ b/arch/arm/boot/dts/exynos4210-i9100.dts
> @@ -560,7 +560,6 @@ charger_reg: CHARGER {
> regulator-name = "CHARGER";
> regulator-min-microamp = <6>;
> regulator-max-microamp = <258>;
> - regulator-always-on;
Thanks for the patch.
The DTS changes always go separately.
> };
>
> chargercv_reg: CHARGER_CV {
> diff --git a/drivers/power/supply/max8997_charger.c
> b/drivers/power/supply/max8997_charger.c
> index 1947af25879a..26cd271576ec 100644
> --- a/drivers/power/supply/max8997_charger.c
> +++ b/drivers/power/supply/max8997_charger.c
> @@ -6,6 +6,7 @@
> // MyungJoo Ham
>
> #include
> +#include
> #include
> #include
> #include
> @@ -31,6 +32,12 @@ struct charger_data {
> struct device *dev;
> struct max8997_dev *iodev;
> struct power_supply *battery;
> + struct regulator *reg;
You need to include regulator consumer.h.
> + struct {
It makes all dereferences longer. Just add a comment that these are
related to the extcon.
> + struct extcon_dev *edev;
> + struct notifier_block nb;
> + struct work_struct work;
> + } extcon;
> };
>
> static enum power_supply_property max8997_battery_props[] = {
> @@ -88,6 +95,63 @@ 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);
> + int ret, current_limit;
> + struct extcon_dev *edev = charger->extcon.edev;
> +
It would be useful to report the current with POWER_SUPPLY_PROP_* but
it is a different patch.
> + 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;
The charger provides 500 mA, so I wonder whether 650 here is correct. Is
it at different voltage?
> + } 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 should be declared here.
> + 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);
Failure of setting the current should rather disable the charging.
> + ret = regulator_enable(charger->reg);
> + if (ret)
> + dev_err(charger->dev, "failed to enable regulator:
> %d\n", ret);
> + } else {
ret should be declared here.
> + ret = regulator_disable(charger->reg);
> + if (ret)
> + dev_err(charger->dev, "failed to disable regulator:
> %d\n", ret);
> + }
What about top-off charging?
> +}
> +
> +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);
> +
Re: [PATCH 4/4] ARM: dts: s3c6410: correct SMDK6410 board compatible
On Tue, Nov 17, 2020 at 09:11:06PM +0100, Krzysztof Kozlowski wrote: > The SMDK6410 DTS was incorrectly called mini6410, probably copy-paste > from FriendlyARM Mini6410 board. > > Signed-off-by: Krzysztof Kozlowski > --- > arch/arm/boot/dts/s3c6410-smdk6410.dts | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Applied. Best regards, Krzysztof
Re: [PATCH 2/4] dt-bindings: arm: samsung: document S3C6410-based boards binding
On Tue, Nov 17, 2020 at 09:11:04PM +0100, Krzysztof Kozlowski wrote: > Add bindings for the FriendlyARM Mini6410 and Samsung SMDK6410 boards. > > Signed-off-by: Krzysztof Kozlowski > --- > .../devicetree/bindings/arm/samsung/samsung-boards.yaml| 7 +++ > 1 file changed, 7 insertions(+) Applied. Best regards, Krzysztof
Re: [PATCH 3/4] ARM: dts: s3c24xx: add SMDK2416 board compatible
On Tue, Nov 17, 2020 at 09:11:05PM +0100, Krzysztof Kozlowski wrote: > Add a compatible for SMDK2416 board next to the SoC compatible. > > Signed-off-by: Krzysztof Kozlowski > --- > arch/arm/boot/dts/s3c2416-smdk2416.dts | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Applied. Best regards, Krzysztof
Re: [PATCH 1/4] dt-bindings: arm: samsung: document SMDK2416 board binding
On Tue, Nov 17, 2020 at 09:11:03PM +0100, Krzysztof Kozlowski wrote: > Add binding for the SMDK2416 board. > > Signed-off-by: Krzysztof Kozlowski > --- > .../devicetree/bindings/arm/samsung/samsung-boards.yaml | 6 ++ > 1 file changed, 6 insertions(+) Applied. Best regards, Krzysztof
[PATCH 1/2] soc: samsung: exynos-chipid: order list of SoCs by name
Bring some order to the list of SoCs. No functional change.
Signed-off-by: Krzysztof Kozlowski
---
drivers/soc/samsung/exynos-chipid.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/soc/samsung/exynos-chipid.c
b/drivers/soc/samsung/exynos-chipid.c
index 8d4d05086906..b4cd0cc00f45 100644
--- a/drivers/soc/samsung/exynos-chipid.c
+++ b/drivers/soc/samsung/exynos-chipid.c
@@ -20,6 +20,7 @@ static const struct exynos_soc_id {
const char *name;
unsigned int id;
} soc_ids[] = {
+ /* List ordered by SoC name */
{ "EXYNOS3250", 0xE3472000 },
{ "EXYNOS4210", 0x4320 }, /* EVT0 revision */
{ "EXYNOS4210", 0x4321 },
@@ -29,10 +30,10 @@ static const struct exynos_soc_id {
{ "EXYNOS5260", 0xE526 },
{ "EXYNOS5410", 0xE541 },
{ "EXYNOS5420", 0xE542 },
+ { "EXYNOS5433", 0xE5433000 },
{ "EXYNOS5440", 0xE544 },
{ "EXYNOS5800", 0xE5422000 },
{ "EXYNOS7420", 0xE742 },
- { "EXYNOS5433", 0xE5433000 },
};
static const char * __init product_id_to_soc_id(unsigned int product_id)
--
2.25.1
[PATCH 2/2] soc: samsung: exynos-chipid: initialize later - with arch_initcall
The Exynos ChipID driver on Exynos SoCs has only informational
purpose - to expose the SoC device in sysfs. No other drivers
depend on it so there is really no benefit of initializing it early.
Instead, initialize everything with arch_initcall which:
1. Allows to use dev_info() as the SoC bus is present (since
core_initcall),
2. Could speed things up because of execution in a SMP environment
(after bringing up secondary CPUs, unlike early_initcall),
3. Reduces the amount of work to be done early, when the kernel has to
bring up critical devices.
Signed-off-by: Krzysztof Kozlowski
---
drivers/soc/samsung/exynos-chipid.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/soc/samsung/exynos-chipid.c
b/drivers/soc/samsung/exynos-chipid.c
index b4cd0cc00f45..1a76eade2ed6 100644
--- a/drivers/soc/samsung/exynos-chipid.c
+++ b/drivers/soc/samsung/exynos-chipid.c
@@ -99,9 +99,9 @@ static int __init exynos_chipid_early_init(void)
goto err;
}
- /* it is too early to use dev_info() here (soc_dev is NULL) */
- pr_info("soc soc0: Exynos: CPU[%s] PRO_ID[0x%x] REV[0x%x] Detected\n",
- soc_dev_attr->soc_id, product_id, revision);
+ dev_info(soc_device_to_device(soc_dev),
+"Exynos: CPU[%s] PRO_ID[0x%x] REV[0x%x] Detected\n",
+soc_dev_attr->soc_id, product_id, revision);
return 0;
@@ -111,4 +111,4 @@ static int __init exynos_chipid_early_init(void)
return ret;
}
-early_initcall(exynos_chipid_early_init);
+arch_initcall(exynos_chipid_early_init);
--
2.25.1
Re: [PATCH v2] arm64: defconfig: Enable REGULATOR_PF8X00
On Thu, Dec 03, 2020 at 01:11:49AM +0530, Jagan Teki wrote: > Enable PF8X00 regulator driver by default as it used in some of > i.MX8MM hardware platforms like Engicam i.Core MX8M Mini SoM. > > Cc: Catalin Marinas > Cc: Will Deacon > Signed-off-by: Jagan Teki You will have to send v2 of entire patchset, so wait with this till you have everything ready. Best regards, Krzysztof > --- > Changes for v2: > - update the commit message.
Re: [PATCH 04/10] arm64: dts: imx8mm: Add Engicam i.Core MX8M Mini SOM
On Thu, Dec 03, 2020 at 01:00:59AM +0530, Jagan Teki wrote: > Hi Krzysztof, > > On Wed, Dec 2, 2020 at 11:04 PM Krzysztof Kozlowski wrote: > > > > On Wed, Dec 02, 2020 at 05:42:35PM +0530, Jagan Teki wrote: > > > i.Core MX8M Mini is an EDIMM SOM based on NXP i.MX8MM from Engicam. > > > > s/SOM/SoM/ > > > > > > > > General features: > > > - NXP i.MX8MM > > > > i.MX 8M Mini > > as named by NXP: > > 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 > > > > > - 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.MX8MM features > > > > Ditto > > > > > > > > i.Core MX8M Mini needs to mount on top of Engicam baseboards for > > > creating complete platform boards. > > > > > > Possible baseboards are, > > > - EDIMM2.2 > > > - C.TOUCH 2.0 > > > > Don't describe baseboards. You add here only SoM. > > It's just information on how this SoM is being used. Let me know any > issues while explaining the combinations being used here. Don't describe baseboards. No point to blow up description. Include only information relevant to this patch. Best regards, Krzysztof
Re: [PATCH 08/10] arm64: dts: imx8mm: Add Engicam i.Core MX8M Mini C.TOUCH 2.0
On Thu, Dec 03, 2020 at 12:50:37AM +0530, Jagan Teki wrote: > Hi Krzysztof, > > On Wed, Dec 2, 2020 at 11:15 PM Krzysztof Kozlowski wrote: > > > > On Wed, Dec 02, 2020 at 05:42:39PM +0530, Jagan Teki wrote: > > > i.Core MX8M Mini is an EDIMM SOM based on NXP i.MX8MM 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 support for it. > > > > > > Signed-off-by: Matteo Lisi > > > Signed-off-by: Jagan Teki > > > --- > > > arch/arm64/boot/dts/freescale/Makefile| 1 + > > > .../imx8mm-engicam-icore-mx8mm-ctouch2.dts| 21 +++ > > > 2 files changed, 22 insertions(+) > > > create mode 100644 > > > arch/arm64/boot/dts/freescale/imx8mm-engicam-icore-mx8mm-ctouch2.dts > > > > > > diff --git a/arch/arm64/boot/dts/freescale/Makefile > > > b/arch/arm64/boot/dts/freescale/Makefile > > > index 4369d783dade..8191db4c64fa 100644 > > > --- a/arch/arm64/boot/dts/freescale/Makefile > > > +++ b/arch/arm64/boot/dts/freescale/Makefile > > > @@ -30,6 +30,7 @@ dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-lx2160a-rdb.dtb > > > dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-lx2162a-qds.dtb > > > > > > dtb-$(CONFIG_ARCH_MXC) += imx8mm-beacon-kit.dtb > > > +dtb-$(CONFIG_ARCH_MXC) += imx8mm-engicam-icore-mx8mm-ctouch2.dtb > > > dtb-$(CONFIG_ARCH_MXC) += imx8mm-engicam-icore-mx8mm-edimm2.2.dtb > > > dtb-$(CONFIG_ARCH_MXC) += imx8mm-evk.dtb > > > dtb-$(CONFIG_ARCH_MXC) += imx8mm-ddr4-evk.dtb > > > diff --git > > > a/arch/arm64/boot/dts/freescale/imx8mm-engicam-icore-mx8mm-ctouch2.dts > > > b/arch/arm64/boot/dts/freescale/imx8mm-engicam-icore-mx8mm-ctouch2.dts > > > new file mode 100644 > > > index ..aa3c03ad3109 > > > --- /dev/null > > > +++ b/arch/arm64/boot/dts/freescale/imx8mm-engicam-icore-mx8mm-ctouch2.dts > > > @@ -0,0 +1,21 @@ > > > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) > > > +/* > > > + * Copyright (c) 2019 NXP > > > + * Copyright (c) 2019 Engicam srl > > > + * Copyright (c) 2020 Amarula Solutions(India) > > > + */ > > > + > > > +/dts-v1/; > > > +#include "imx8mm.dtsi" > > > > You have multiple DTSI files to only include one DTSI. I was trying to > > follow the logic here but I failed... > > > > This is ctouch, so it should include SoM, which you call icore. But it > > also includes ctouch2 which *only* includes common DTSI. It's then > > exactly the same as starter kit which includes edimm (which includes > > common) and icore. > > I hope you have checked the cover letter where I have mentioned all > the combinations. > > 1. SoM, Starter Kit, Carrier Board, Open Frame are three different hardware. > > 2. i.Core MX8M Mini is SoM > > 3. EDIMM 2.2 is Starter Kit > > 4. C.TOUCH 2.0 is Carrier board > > 5. 10"1 Open Frame board for LVDS > > The combination of respective hardware mounting is, > > 1. SOM+Starter Kitt => i.Core MX8M Mini EDIMM 2.2 Starter Kit > > 2. SOM+C.TOUCH 2.0 => i.Core MX8M Mini C.TOUCH 2.0 Carrier board > > 3. SOM+C.TOUCH 2.0+10.1" OF => i.Core MX8M Mini C.TOUCH 2.0 10.1" Open > Frame board It does not explain why you created 3 empty DTSI and 2 empty DTS files. > > About the bindings, (please check the > arch/arm64/boot/dts/rockchip/px30-engicam-*), It's been discussed > before with Rob for these boards bindings. Refer to my specific comments about bindings. > > To, compare with what we have described with rockchip > > SoM binding, > - engicam,icore-mx8mm is binding for i.Core MX8M Mini SoM > - engicam,px30-core is binding for PX30.Core SoM > > EDIMM 2.2 is Starter Kit binding, > - engicam,icore-mx8mm-edimm2.2 is binding for EDIMM 2.2 is Starter Kit > in i.MX8MM > - engicam,px30-core-edimm2.2 is binding for EDIMM 2.2 is Starter Kit in PX30 > > C.TOUCH 2.0 is Carrier board binding, > - engicam,icore-mx8mm-ctouch2 is binding for C.TOUCH 2.0 is Carrier > board in i.MX8MM > - engicam,px30-core-ctouch2 is binding for C.TOUCH 2.0 is Carrier board in > PX30 > > C.TOUCH 2.0 10"1 OF binding, > - engicam,icore-mx8mm-ctouch2-of10 is binding for C.TOUCH 2.0 10"1 in imx8MM > - engicam,px30-core-ctouch2-of10 for C.TOUCH 2.0 10"1 in PX30 > > So, there
Re: [PATCH 10/10] arm64: dts: imx8mm: Add Engicam i.Core MX8M Mini C.TOUCH 2.0 10.1" OF
On Wed, Dec 02, 2020 at 05:42:41PM +0530, Jagan Teki wrote:
> i.Core MX8M Mini is an EDIMM SOM based on NXP i.MX8MM from Engicam.
>
> C.TOUCH 2.0 is a general purpose carrier board with capacitive
> touch interface support.
>
> 10.1" OF is a capacitive touch 10.1" Open Frame panel solutions.
>
> i.Core MX8M Mini needs to mount on top of C.TOUCH 2.0 carrier with
> pluged 10.1" OF for creating complete i.Core MX8M Mini C.TOUCH 2.0
> 10.1" Open Frame solution board.
>
> Add support for it.
>
> Signed-off-by: Matteo Lisi
> Signed-off-by: Jagan Teki
> ---
> arch/arm64/boot/dts/freescale/Makefile| 1 +
> ...mx8mm-engicam-icore-mx8mm-ctouch2-of10.dts | 21 +++
> 2 files changed, 22 insertions(+)
> create mode 100644
> arch/arm64/boot/dts/freescale/imx8mm-engicam-icore-mx8mm-ctouch2-of10.dts
>
> diff --git a/arch/arm64/boot/dts/freescale/Makefile
> b/arch/arm64/boot/dts/freescale/Makefile
> index 8191db4c64fa..9725bbc0f268 100644
> --- a/arch/arm64/boot/dts/freescale/Makefile
> +++ b/arch/arm64/boot/dts/freescale/Makefile
> @@ -31,6 +31,7 @@ dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-lx2162a-qds.dtb
>
> dtb-$(CONFIG_ARCH_MXC) += imx8mm-beacon-kit.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mm-engicam-icore-mx8mm-ctouch2.dtb
> +dtb-$(CONFIG_ARCH_MXC) += imx8mm-engicam-icore-mx8mm-ctouch2-of10.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mm-engicam-icore-mx8mm-edimm2.2.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mm-evk.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mm-ddr4-evk.dtb
> diff --git
> a/arch/arm64/boot/dts/freescale/imx8mm-engicam-icore-mx8mm-ctouch2-of10.dts
> b/arch/arm64/boot/dts/freescale/imx8mm-engicam-icore-mx8mm-ctouch2-of10.dts
> new file mode 100644
> index ..0124ba5ec69c
> --- /dev/null
> +++
> b/arch/arm64/boot/dts/freescale/imx8mm-engicam-icore-mx8mm-ctouch2-of10.dts
> @@ -0,0 +1,21 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (c) 2019 NXP
> + * Copyright (c) 2019 Engicam srl
> + * Copyright (c) 2020 Amarula Solutions(India)
> + */
> +
> +/dts-v1/;
> +#include "imx8mm.dtsi"
> +#include "imx8mm-engicam-ctouch2.dtsi"
> +#include "imx8mm-engicam-icore-mx8mm.dtsi"
> +
> +/ {
> + model = "Engicam i.Core MX8M Mini C.TOUCH 2.0 10.1\" Open Frame";
> + compatible = "engicam,icore-mx8mm-ctouch2-of10", "engicam,icore-mx8mm",
> + "fsl,imx8mm";
No. You created 3 DTS and 3 DTSI which are all the same. The output DTB
is probably the same for all three DTS files. Only one DTSI has anything
inside, all others are just copy paste.
Best regards,
Krzysztof
Re: [PATCH 08/10] arm64: dts: imx8mm: Add Engicam i.Core MX8M Mini C.TOUCH 2.0
On Wed, Dec 02, 2020 at 05:42:39PM +0530, Jagan Teki wrote:
> i.Core MX8M Mini is an EDIMM SOM based on NXP i.MX8MM 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 support for it.
>
> Signed-off-by: Matteo Lisi
> Signed-off-by: Jagan Teki
> ---
> arch/arm64/boot/dts/freescale/Makefile| 1 +
> .../imx8mm-engicam-icore-mx8mm-ctouch2.dts| 21 +++
> 2 files changed, 22 insertions(+)
> create mode 100644
> arch/arm64/boot/dts/freescale/imx8mm-engicam-icore-mx8mm-ctouch2.dts
>
> diff --git a/arch/arm64/boot/dts/freescale/Makefile
> b/arch/arm64/boot/dts/freescale/Makefile
> index 4369d783dade..8191db4c64fa 100644
> --- a/arch/arm64/boot/dts/freescale/Makefile
> +++ b/arch/arm64/boot/dts/freescale/Makefile
> @@ -30,6 +30,7 @@ dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-lx2160a-rdb.dtb
> dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-lx2162a-qds.dtb
>
> dtb-$(CONFIG_ARCH_MXC) += imx8mm-beacon-kit.dtb
> +dtb-$(CONFIG_ARCH_MXC) += imx8mm-engicam-icore-mx8mm-ctouch2.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mm-engicam-icore-mx8mm-edimm2.2.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mm-evk.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mm-ddr4-evk.dtb
> diff --git
> a/arch/arm64/boot/dts/freescale/imx8mm-engicam-icore-mx8mm-ctouch2.dts
> b/arch/arm64/boot/dts/freescale/imx8mm-engicam-icore-mx8mm-ctouch2.dts
> new file mode 100644
> index ..aa3c03ad3109
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx8mm-engicam-icore-mx8mm-ctouch2.dts
> @@ -0,0 +1,21 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (c) 2019 NXP
> + * Copyright (c) 2019 Engicam srl
> + * Copyright (c) 2020 Amarula Solutions(India)
> + */
> +
> +/dts-v1/;
> +#include "imx8mm.dtsi"
You have multiple DTSI files to only include one DTSI. I was trying to
follow the logic here but I failed...
This is ctouch, so it should include SoM, which you call icore. But it
also includes ctouch2 which *only* includes common DTSI. It's then
exactly the same as starter kit which includes edimm (which includes
common) and icore.
Best regards,
Krzysztof
> +#include "imx8mm-engicam-ctouch2.dtsi"
> +#include "imx8mm-engicam-icore-mx8mm.dtsi"
> +
> +/ {
Re: [PATCH 09/10] dt-bindings: arm: fsl: Add Engicam i.Core MX8M Mini C.TOUCH 2.0 10.1" OF
On Wed, Dec 02, 2020 at 05:42:40PM +0530, Jagan Teki wrote: > i.Core MX8M Mini is an EDIMM SOM based on NXP i.MX8MM from Engicam. > > C.TOUCH 2.0 is a general purpose carrier board with capacitive > touch interface support. > > 10.1" OF is a capacitive touch 10.1" Open Frame panel solutions. > > i.Core MX8M Mini needs to mount on top of C.TOUCH 2.0 carrier with > pluged 10.1" OF for creating complete i.Core MX8M Mini C.TOUCH 2.0 > 10.1" Open Frame solution board. > > Add bindings for it. > > Signed-off-by: Jagan Teki Please run the checkpatch - it should complain about order of patches. > --- > Documentation/devicetree/bindings/arm/fsl.yaml | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml > b/Documentation/devicetree/bindings/arm/fsl.yaml > index 8c8f7728788d..9e275921112d 100644 > --- a/Documentation/devicetree/bindings/arm/fsl.yaml > +++ b/Documentation/devicetree/bindings/arm/fsl.yaml > @@ -669,6 +669,7 @@ properties: >- 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 > + - engicam,icore-mx8mm-ctouch2-of10 # i.MX8MM Engicam i.Core > MX8M Mini C.TOUCH 2.0 10.1" Open Frame Run dtbs_check before posting. Best regards, Krzysztof
Re: [PATCH 06/10] arm64: dts: imx: Add Engicam C.TOUCH 2.0
On Wed, Dec 02, 2020 at 05:42:37PM +0530, Jagan Teki wrote: > Engicam C.TOUCH 2.0 is an EDIMM compliant general purpose > carrier board with capacitive touch interface. > > Genaral features: > - TFT 10.1" industrial, 1280x800 LVDS display > - Ethernet 10/100 > - Wifi/BT > - USB Type A/OTG > - Audio Out > - CAN > - LVDS panel connector > > SOM's like 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 > --- > arch/arm64/boot/dts/freescale/imx8mm-engicam-ctouch2.dtsi | 7 +++ > 1 file changed, 7 insertions(+) > create mode 100644 arch/arm64/boot/dts/freescale/imx8mm-engicam-ctouch2.dtsi > > 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 Amarula Solutions(India) > + */ > + > +#include "imx8mm-engicam-common.dtsi" I don't see any point of this DTS. You have a DTSI to include a DTSI. Please describe in details your DTS architecture... Best regards, Krzysztof
Re: [PATCH 05/10] arm64: dts: imx8mm: Add Engicam i.Core MX8M Mini EDIMM2.2 Starter Kit
On Wed, Dec 02, 2020 at 05:42:36PM +0530, Jagan Teki wrote:
> i.Core MX8M Mini is an EDIMM SOM based on NXP i.MX8MM from Engicam.
>
> EDIMM2.2 Starter Kit is an EDIMM 2.2 Form Factor Capacitive
> Evaluation Board 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.
>
> Add support for it.
>
> Signed-off-by: Matteo Lisi
> Signed-off-by: Jagan Teki
> ---
> arch/arm64/boot/dts/freescale/Makefile| 1 +
> .../imx8mm-engicam-icore-mx8mm-edimm2.2.dts | 21 +++
> 2 files changed, 22 insertions(+)
> create mode 100644
> arch/arm64/boot/dts/freescale/imx8mm-engicam-icore-mx8mm-edimm2.2.dts
>
> diff --git a/arch/arm64/boot/dts/freescale/Makefile
> b/arch/arm64/boot/dts/freescale/Makefile
> index 6f0777ee6cd6..4369d783dade 100644
> --- a/arch/arm64/boot/dts/freescale/Makefile
> +++ b/arch/arm64/boot/dts/freescale/Makefile
> @@ -30,6 +30,7 @@ dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-lx2160a-rdb.dtb
> dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-lx2162a-qds.dtb
>
> dtb-$(CONFIG_ARCH_MXC) += imx8mm-beacon-kit.dtb
> +dtb-$(CONFIG_ARCH_MXC) += imx8mm-engicam-icore-mx8mm-edimm2.2.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mm-evk.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mm-ddr4-evk.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mm-kontron-n801x-s.dtb
> diff --git
> a/arch/arm64/boot/dts/freescale/imx8mm-engicam-icore-mx8mm-edimm2.2.dts
> b/arch/arm64/boot/dts/freescale/imx8mm-engicam-icore-mx8mm-edimm2.2.dts
> new file mode 100644
> index ..a8afc0998fcd
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx8mm-engicam-icore-mx8mm-edimm2.2.dts
> @@ -0,0 +1,21 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (c) 2019 NXP
> + * Copyright (c) 2019 Engicam srl
> + * Copyright (c) 2020 Amarula Solutions(India)
> + */
> +
> +/dts-v1/;
> +#include "imx8mm.dtsi"
imx8mm should be included in the SoM, not here. It's really wrong that
you override some nodes in imx8mm-engicam-icore-mx8mm.dtsi which do not
exist there (not included).
> +#include "imx8mm-engicam-edimm2.2.dtsi"
> +#include "imx8mm-engicam-icore-mx8mm.dtsi"
> +
> +/ {
> + model = "Engicam i.Core MX8M Mini EDIMM2.2 Starter Kit";
> + compatible = "engicam,icore-mx8mm-edimm2.2", "engicam,icore-mx8mm",
This won't validate against your own bindings. Please fix either the DTS
or the bindings. Then run dtbs_check.
Best regards,
Krzysztof
Re: [PATCH 04/10] arm64: dts: imx8mm: Add Engicam i.Core MX8M Mini SOM
On Wed, Dec 02, 2020 at 05:42:35PM +0530, Jagan Teki wrote:
> i.Core MX8M Mini is an EDIMM SOM based on NXP i.MX8MM from Engicam.
s/SOM/SoM/
>
> General features:
> - NXP i.MX8MM
i.MX 8M Mini
as named by NXP:
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
> - 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.MX8MM features
Ditto
>
> i.Core MX8M Mini needs to mount on top of Engicam baseboards for
> creating complete platform boards.
>
> Possible baseboards are,
> - EDIMM2.2
> - C.TOUCH 2.0
Don't describe baseboards. You add here only SoM.
>
> Add support for it.
>
> Signed-off-by: Matteo Lisi
> Signed-off-by: Jagan Teki
> ---
> .../freescale/imx8mm-engicam-icore-mx8mm.dtsi | 209 ++
> 1 file changed, 209 insertions(+)
> create mode 100644
> arch/arm64/boot/dts/freescale/imx8mm-engicam-icore-mx8mm.dtsi
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8mm-engicam-icore-mx8mm.dtsi
> b/arch/arm64/boot/dts/freescale/imx8mm-engicam-icore-mx8mm.dtsi
> new file mode 100644
> index ..b87917c40587
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx8mm-engicam-icore-mx8mm.dtsi
> @@ -0,0 +1,209 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (c) 2018 NXP
> + * Copyright (c) 2019 Engicam srl
> + * Copyright (c) 2020 Amarula Solutons(India)
> + */
> +
> +/ {
Missing "model".
> + compatible = "engicam,icore-mx8mm", "fsl,imx8mm";
> +};
> +
No memory node? Isn't the memory a property of SoM?
> +_0 {
> + cpu-supply = <_buck4>;
> +};
Supplies for the other cores.
> +
> + {
> + clock-frequency = <40>;
> + pinctrl-names = "default";
> + pinctrl-0 = <_i2c1>;
> + status = "okay";
> +
> + pf8100@8 {
Node name should describe generic class of a device, so probably you
wanted here "pmic".
> + compatible = "nxp,pf8x00";
> + reg = <0x08>;
> +
> + regulators {
> + reg_ldo1: ldo1 {
> + regulator-always-on;
> + regulator-boot-on;
First min/max constraints. Then always-on and boot-on properties.
> + regulator-max-microvolt = <500>;
> + regulator-min-microvolt = <150>;
> + };
> +
> + reg_ldo2: ldo2 {
> + regulator-always-on;
> + regulator-boot-on;
> + regulator-max-microvolt = <500>;
> + regulator-min-microvolt = <150>;
> + };
> +
> + reg_ldo3: ldo3 {
> + regulator-always-on;
> + regulator-boot-on;
> + regulator-max-microvolt = <500>;
> + regulator-min-microvolt = <150>;
> + };
> +
> + reg_ldo4: ldo4 {
> + regulator-always-on;
> + regulator-boot-on;
> + regulator-max-microvolt = <500>;
> + regulator-min-microvolt = <150>;
> + };
> +
> + reg_buck1: buck1 {
> + fsl,ilim-ma = <4500>;
Where is this property documented?
> + regulator-always-on;
> + regulator-boot-on;
> + regulator-max-microvolt = <180>;
> + regulator-min-microvolt = <40>;
> + };
> +
> + reg_buck2: buck2 {
> + regulator-always-on;
> + regulator-boot-on;
> + regulator-max-microvolt = <180>;
> + regulator-min-microvolt = <40>;
> + };
> +
> + reg_buck3: buck3 {
> + regulator-always-on;
> + regulator-boot-on;
> + regulator-max-microvolt = <180>;
> + regulator-min-microvolt = <40>;
> + };
> +
> + reg_buck4: buck4 {
> + regulator-always-on;
> + regulator-boot-on;
> + regulator-max-microvolt = <180>;
> + regulator-min-microvolt = <40>;
> + fast-slew = <1>;
Where is this property documented?
> + };
> +
> + reg_buck5: buck5 {
> +
Re: [PATCH 03/10] arm64: dts: imx: Add Engicam EDIMM2.2 Starter Kit
On Wed, Dec 02, 2020 at 05:42:34PM +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
>
> SOM's like i.Core MX8M Mini needs to mount on top of this Evaluation
> board for creating complete i.Core MX8M Mini EDIMM2.2 Starter Kit.
>
> Add support for it.
>
> Signed-off-by: Matteo Lisi
> Signed-off-by: Jagan Teki
> ---
> .../dts/freescale/imx8mm-engicam-common.dtsi | 24 +++
> .../freescale/imx8mm-engicam-edimm2.2.dtsi| 7 ++
> 2 files changed, 31 insertions(+)
> create mode 100644 arch/arm64/boot/dts/freescale/imx8mm-engicam-common.dtsi
> create mode 100644 arch/arm64/boot/dts/freescale/imx8mm-engicam-edimm2.2.dtsi
>
> 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 ..67c1a3fe26bc
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx8mm-engicam-common.dtsi
> @@ -0,0 +1,24 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (c) 2020 Engicam srl
> + * Copyright (c) 2020 Amarula Solutions(India)
> + */
> +
> + {
> + 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-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"
A DTSI file only with UART and SD. You mentioned several features in the
commit msg but none of them are implemented here. There aren't even
CPUs... Maybe this is only a problem of your patch ordering but as of
now - this looks like bogus/empty DTSI which should not be in its own
commit.
Best regards,
Krzysztof
Re: [PATCH 07/10] dt-bindings: arm: fsl: Add Engicam i.Core MX8M Mini C.TOUCH 2.0
On Wed, Dec 02, 2020 at 05:42:38PM +0530, Jagan Teki wrote: > i.Core MX8M Mini is an EDIMM SOM based on NXP i.MX8MM 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. This should be squashed with previous one. You basically add support for both similar boards so there is no reason to split the bindings change into two. Best regards, Krzysztof
Re: [PATCH 01/10] arm64: defconfig: Enable REGULATOR_PF8X00
On Wed, Dec 02, 2020 at 05:42:32PM +0530, Jagan Teki wrote: > Enable PF8X00 regulator driver by default as it used in > some of i.MX8MM hardware platforms. Could you mention names (one is enough) of platforms this could be found on? This would be more detailed reason why the option should be enabled. Best regards, Krzysztof > > Cc: Catalin Marinas > Cc: Will Deacon > Signed-off-by: Jagan Teki > --- > arch/arm64/configs/defconfig | 1 + > 1 file changed, 1 insertion(+)
Re: [PATCH v5 net-next 1/4] dt-bindings: net: nfc: s3fwrn5: Support a UART interface
On Wed, Dec 02, 2020 at 08:47:38PM +0900, Bongsu Jeon wrote: > From: Bongsu Jeon > > Since S3FWRN82 NFC Chip, The UART interface can be used. > S3FWRN82 supports I2C and UART interface. > > Signed-off-by: Bongsu Jeon > --- > .../bindings/net/nfc/samsung,s3fwrn5.yaml | 31 > +++--- > 1 file changed, 28 insertions(+), 3 deletions(-) > Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH v2 7/7] arm64: defconfig: Enable interconnect for imx8mq
On Wed, 2 Dec 2020 at 13:30, Martin Kepplinger wrote: > > -CONFIG_INTERCONNECT=y > > Why are you removing this line? > >>> > >>> savedefconfig removes it. INTERCONNECT_IMX below depends on it. > >> > >> It's save to remove it as other Interconnect options are directly > >> dependant. > > > > Ugh, my bad, it is not allowed to remove it. My review was too fast. > > INTERCONNECT_IMX depends on it, so the INTERCONNECT must stay, > > > > It is selected by TEGRA_MC which is independent here, so you should keep it. > > > > thanks for reviewing! Just to be clear: We're talking about defconfig, > so if I keep INTERCONNECT that means that I do `make savedefconfig` > which removes it (it's of course still enabled, just redundant in > defconfig output), and then *manually* add INTERCONNECT. That would > indicate that there's a Kconfig bug. I don't get the point. You should not send the savedefconfig output as is, without any adjustments and checks. You can run savedefconfig because it nicely puts your entries in the proper place, but it's not a bug that it removes features which we *want to keep*. Where is a bug in Kconfig? Best regards, Krzysztof
Re: [PATCH v2 1/7] arm64: dts: imx8m: Add NOC nodes
On Tue, Dec 01, 2020 at 01:39:26PM +0100, Martin Kepplinger wrote:
> From: Leonard Crestez
>
> Add initial support for dynamic frequency scaling of main NOC.
>
> Make DDRC the parent of the NOC (using passive governor) so that the
> main NOC is automatically scaled together with DDRC by default.
>
> Support for proactive scaling via interconnect will come on top.
>
> Signed-off-by: Leonard Crestez
> Tested-by: Martin Kepplinger (imx8mq)
> ---
> arch/arm64/boot/dts/freescale/imx8mm.dtsi | 22 ++
> arch/arm64/boot/dts/freescale/imx8mn.dtsi | 22 ++
> arch/arm64/boot/dts/freescale/imx8mq.dtsi | 22 ++
> 3 files changed, 66 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> index c824f2615fe8..835b19f0ea42 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> @@ -921,6 +921,28 @@
>
> };
>
> + noc: interconnect@3270 {
> + compatible = "fsl,imx8mm-noc", "fsl,imx8m-noc";
> + reg = <0x3270 0x10>;
> + clocks = < IMX8MM_CLK_NOC>;
> + devfreq = <>;
This does not pass the dtschema checks. Are you missing here any
dependencies?
arch/arm64/boot/dts/freescale/imx8mm-evk.dt.yaml: interconnect@3270:
'devfreq' does not match any of the regexes: 'pinctrl-[0-9]+'
Best regards,
Krzysztof
Re: [PATCH v2 7/7] arm64: defconfig: Enable interconnect for imx8mq
On Wed, 2 Dec 2020 at 10:03, Krzysztof Kozlowski wrote: > > On Tue, Dec 01, 2020 at 02:15:04PM +0100, Martin Kepplinger wrote: > > On 01.12.20 14:10, Georgi Djakov wrote: > > > On 1.12.20 14:39, Martin Kepplinger wrote: > > > > Enable INTERCONNECT_IMX8MQ in order to make interconnect more widely > > > > available for testing. > > > > > > I hope that it's not just for testing, but using it. > > > > sure, I just think that most people will use their own config for production > > but that's a different story. I can rephrase. > > > > > > > > > Signed-off-by: Martin Kepplinger > > > > --- > > > > arch/arm64/configs/defconfig | 3 ++- > > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig > > > > index 1fed16950a7c..830c26a95b3d 100644 > > > > --- a/arch/arm64/configs/defconfig > > > > +++ b/arch/arm64/configs/defconfig > > > > @@ -1023,7 +1023,8 @@ CONFIG_OPTEE=y > > > > CONFIG_MUX_MMIO=y > > > > CONFIG_SLIM_QCOM_CTRL=m > > > > CONFIG_SLIM_QCOM_NGD_CTRL=m > > > > -CONFIG_INTERCONNECT=y > > > > > > Why are you removing this line? > > > > savedefconfig removes it. INTERCONNECT_IMX below depends on it. > > It's save to remove it as other Interconnect options are directly > dependant. Ugh, my bad, it is not allowed to remove it. My review was too fast. INTERCONNECT_IMX depends on it, so the INTERCONNECT must stay, It is selected by TEGRA_MC which is independent here, so you should keep it. Best regards, Krzysztof
Re: [PATCH v2 7/7] arm64: defconfig: Enable interconnect for imx8mq
On Tue, Dec 01, 2020 at 02:15:04PM +0100, Martin Kepplinger wrote: > On 01.12.20 14:10, Georgi Djakov wrote: > > On 1.12.20 14:39, Martin Kepplinger wrote: > > > Enable INTERCONNECT_IMX8MQ in order to make interconnect more widely > > > available for testing. > > > > I hope that it's not just for testing, but using it. > > sure, I just think that most people will use their own config for production > but that's a different story. I can rephrase. > > > > > > Signed-off-by: Martin Kepplinger > > > --- > > > arch/arm64/configs/defconfig | 3 ++- > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig > > > index 1fed16950a7c..830c26a95b3d 100644 > > > --- a/arch/arm64/configs/defconfig > > > +++ b/arch/arm64/configs/defconfig > > > @@ -1023,7 +1023,8 @@ CONFIG_OPTEE=y > > > CONFIG_MUX_MMIO=y > > > CONFIG_SLIM_QCOM_CTRL=m > > > CONFIG_SLIM_QCOM_NGD_CTRL=m > > > -CONFIG_INTERCONNECT=y > > > > Why are you removing this line? > > savedefconfig removes it. INTERCONNECT_IMX below depends on it. It's save to remove it as other Interconnect options are directly dependant. Best regards, Krzysztof
Re: [PATCH v2 7/7] arm64: defconfig: Enable interconnect for imx8mq
On Tue, Dec 01, 2020 at 01:39:32PM +0100, Martin Kepplinger wrote: > Enable INTERCONNECT_IMX8MQ in order to make interconnect more widely > available for testing. > > Signed-off-by: Martin Kepplinger > --- > arch/arm64/configs/defconfig | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [RFC 2/2] MAINTAINERS: add a limited ARM and ARM64 SoC entry
On Tue, Dec 01, 2020 at 11:15:16PM +0200, Krzysztof Kozlowski wrote: > It is expected for ARM and ARM64 SoC related code to go through > sub-architecture maintainers. Their addresses were therefore not I reshaped my message last moment and missed that it does not make sense anymore... It should be: "The ARM and ARM64 SoC maintainers addresses were therefore not..." Best regards, Krzysztof > documented to push patch traffic through sub-architecture maintainers. > > However when patches touch generic code, e.g. multi_v7_defconfig, the > patch might not be picked up by them and instead should go to the SoC > maintainers - Arnd and Olof. > > Add a minimal maintainer's entry for SoC covering only Makefile, so it > will not appear on most of submissions (except new devicetree boards). > It will though serve as a documentation and reference for cases when > submitter does not know where to send his SoC-related patches. > > Signed-off-by: Krzysztof Kozlowski > --- > MAINTAINERS | 10 ++ > 1 file changed, 10 insertions(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 6149066a545e..f302983645bd 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -1491,6 +1491,16 @@ F: > Documentation/devicetree/bindings/iommu/arm,smmu* > F: drivers/iommu/arm/ > F: drivers/iommu/io-pgtable-arm* > > +ARM AND ARM64 SoC SUB-ARCHITECTURES (COMMON PARTS) > +M: Arnd Bergmann > +M: Olof Johansson > +M: s...@kernel.org > +L: linux-arm-ker...@lists.infradead.org (moderated for non-subscribers) > +S: Maintained > +T: git git://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git > +F: arch/arm/boot/dts/Makefile > +F: arch/arm64/boot/dts/Makefile > + > ARM SUB-ARCHITECTURES > L: linux-arm-ker...@lists.infradead.org (moderated for non-subscribers) > S: Maintained > -- > 2.25.1 >
Re: [PATCH 1/5] ARM: configs: drop unused BACKLIGHT_GENERIC option
On Tue, Dec 01, 2020 at 04:50:22PM +0100, Arnd Bergmann wrote: > On Tue, Dec 1, 2020 at 4:41 PM Alexandre Belloni > wrote: > > On 01/12/2020 14:40:53+, Catalin Marinas wrote: > > > On Mon, Nov 30, 2020 at 07:50:25PM +, ZHIZHIKIN Andrey wrote: > > > > From Krzysztof Kozlowski : > > > > I tried to convince them before, it didn't work. I guess they don't like > > > to be spammed ;). > > > > The first rule of arm-soc is: you do not talk about arm@ and soc@ > > I don't mind having the addresses documented better, but it needs to > be done in a way that avoids having any patch for arch/arm*/boot/dts > and arch/arm/*/configs Cc:d to s...@kernel.org. > > If anyone has suggestions for how to do that, let me know. Not a perfect solution but something. How about: https://lore.kernel.org/linux-arm-kernel/20201201211516.24921-2-k...@kernel.org/T/#u Would not work on defconfigs but there is a chance someone will find your addresses this way. Should not cause to much additional traffic. Best regards, Krzysztof
[GIT PULL] pinctrl: samsung: Pull for v5.11
The following changes since commit 3650b228f83adda7e5ee532e2b90429c03f7b9ec: Linux 5.10-rc1 (2020-10-25 15:14:11 -0700) are available in the Git repository at: https://git.kernel.org/pub/scm/linux/kernel/git/pinctrl/samsung.git tags/samsung-pinctrl-5.11 for you to fetch changes up to c5564a50d99019f3c713fa306d5feecc3e909b10: pinctrl: samsung: s3c24xx: remove unneeded break (2020-10-26 20:23:29 +0100) Samsung pinctrl drivers changes for v5.11 Only a cleanup of unneeded breaks. Tom Rix (1): pinctrl: samsung: s3c24xx: remove unneeded break drivers/pinctrl/samsung/pinctrl-s3c24xx.c | 5 - 1 file changed, 5 deletions(-)
Re: [PATCH v4 net-next 4/4] nfc: s3fwrn5: Support a UART interface
On Tue, Dec 01, 2020 at 10:50:28PM +0900, Bongsu Jeon wrote: > From: Bongsu Jeon > > Since S3FWRN82 NFC Chip, The UART interface can be used. > S3FWRN82 uses NCI protocol and supports I2C and UART interface. > > Signed-off-by: Bongsu Jeon > --- > drivers/nfc/s3fwrn5/Kconfig | 12 +++ > drivers/nfc/s3fwrn5/Makefile | 2 + > drivers/nfc/s3fwrn5/phy_common.c | 12 +++ > drivers/nfc/s3fwrn5/phy_common.h | 1 + > drivers/nfc/s3fwrn5/uart.c | 196 > +++ > 5 files changed, 223 insertions(+) > create mode 100644 drivers/nfc/s3fwrn5/uart.c > Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH v4 net-next 3/4] nfc: s3fwrn5: extract the common phy blocks
On Tue, Dec 01, 2020 at 10:50:27PM +0900, Bongsu Jeon wrote: > From: Bongsu Jeon > > Extract the common phy blocks to reuse it. > The UART module will use the common blocks. > > Signed-off-by: Bongsu Jeon > --- > drivers/nfc/s3fwrn5/Makefile | 2 +- > drivers/nfc/s3fwrn5/i2c.c| 117 > +-- > drivers/nfc/s3fwrn5/phy_common.c | 63 + > drivers/nfc/s3fwrn5/phy_common.h | 36 > 4 files changed, 139 insertions(+), 79 deletions(-) > create mode 100644 drivers/nfc/s3fwrn5/phy_common.c > create mode 100644 drivers/nfc/s3fwrn5/phy_common.h > Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH v4 net-next 1/4] dt-bindings: net: nfc: s3fwrn5: Support a UART interface
On Tue, Dec 01, 2020 at 10:50:25PM +0900, Bongsu Jeon wrote:
> From: Bongsu Jeon
>
> Since S3FWRN82 NFC Chip, The UART interface can be used.
> S3FWRN82 supports I2C and UART interface.
>
> Signed-off-by: Bongsu Jeon
> ---
> .../bindings/net/nfc/samsung,s3fwrn5.yaml | 32
> --
> 1 file changed, 29 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/nfc/samsung,s3fwrn5.yaml
> b/Documentation/devicetree/bindings/net/nfc/samsung,s3fwrn5.yaml
> index cb0b8a5..cc5f9a1 100644
> --- a/Documentation/devicetree/bindings/net/nfc/samsung,s3fwrn5.yaml
> +++ b/Documentation/devicetree/bindings/net/nfc/samsung,s3fwrn5.yaml
> @@ -12,7 +12,10 @@ maintainers:
>
> properties:
>compatible:
> -const: samsung,s3fwrn5-i2c
> +items:
This still has items but it should be a simple enum.
> + - enum:
> + - samsung,s3fwrn5-i2c
> + - samsung,s3fwrn82
>
>en-gpios:
> maxItems: 1
> @@ -47,10 +50,19 @@ additionalProperties: false
> required:
>- compatible
>- en-gpios
> - - interrupts
> - - reg
>- wake-gpios
>
> +allOf:
> + - if:
> + properties:
> +compatible:
> + contains:
> +const: samsung,s3fwrn5-i2c
> +then:
> + required:
> +- interrupts
> +- reg
> +
> examples:
>- |
> #include
> @@ -71,3 +83,17 @@ examples:
> wake-gpios = < 2 GPIO_ACTIVE_HIGH>;
> };
> };
> + # UART example on Raspberry Pi
> + - |
> +uart0 {
> +status = "okay";
> +
> +nfc {
> +compatible = "samsung,s3fwrn82";
> +
> +en-gpios = < 20 0>;
> +wake-gpios = < 16 0>;
Use GPIO flags like in example above.
Best regards,
Krzysztof
Re: [PATCH] phy: samsung: Fix build break in USB2 PHY driver for Exynos5420 SoCs
On Tue, Dec 01, 2020 at 06:09:05PM +0100, Marek Szyprowski wrote:
> Exynos5420 variant of USB2 PHY is handled by the same code as the
> Exynos5250 one. Introducing a separate Kconfig symbol for it was an
> over-engineering, which turned out to cause build break for certain
> configurations:
>
> ERROR: modpost: "exynos5420_usb2_phy_config"
> [drivers/phy/samsung/phy-exynos-usb2.ko] undefined!
>
> Fix this by removing PHY_EXYNOS5420_USB2 symbol and using
> PHY_EXYNOS5250_USB2 also for Exynos5420 SoCs.
>
> Reported-by: Markus Reichl
> Fixes: 81b534f7e9b2 ("phy: samsung: Add support for the Exynos5420 variant of
> the USB2 PHY")
> Signed-off-by: Marek Szyprowski
> ---
> Vinod: this a fix to the patch merged yesterday. If you want me to resend
> a fixed initial patch, let me know.
> ---
> drivers/phy/samsung/Kconfig| 7 +--
> drivers/phy/samsung/phy-samsung-usb2.c | 2 --
> 2 files changed, 1 insertion(+), 8 deletions(-)
>
Acked-by: Krzysztof Kozlowski
Best regards,
Krzysztof
Re: [PATCH 0/5] drop unused BACKLIGHT_GENERIC option
On Mon, Nov 30, 2020 at 08:11:33PM +0100, Sam Ravnborg wrote:
> On Mon, Nov 30, 2020 at 03:21:32PM +, Andrey Zhizhikin wrote:
> > Since the removal of generic_bl driver from the source tree in commit
> > 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is
> > unused") BACKLIGHT_GENERIC config option became obsolete as well and
> > therefore subject to clean-up from all configuration files.
> >
> > This series introduces patches to address this removal, separated by
> > architectures in the kernel tree.
> >
> > Andrey Zhizhikin (5):
> > ARM: configs: drop unused BACKLIGHT_GENERIC option
> > arm64: defconfig: drop unused BACKLIGHT_GENERIC option
> > MIPS: configs: drop unused BACKLIGHT_GENERIC option
> > parisc: configs: drop unused BACKLIGHT_GENERIC option
> > powerpc/configs: drop unused BACKLIGHT_GENERIC option
>
> For defconfigs I expect arch maintainers to do a make xxxdefconfig / make
> savedefconfig / cp defconfig ... run now and then - this will remove
> all such symbols.
savedefconfig can be tricky because of risk of loosing options:
1. it will remove options which became the default or became selected,
2. later when the default is changed or selecting option is removed, the
first option from #1 will not be brought back.
This was already for example with DEBUG_FS and the conclusion that time
was - do not run savedefconfig automatically.
Therefore if some symbol(s) can be safely removed, patch is welcomed.
Best regards,
Krzysztof
>
> If the patches goes in like they are submitted then:
> Acked-by: Sam Ravnborg
Re: [PATCH 5/5] powerpc/configs: drop unused BACKLIGHT_GENERIC option
On Mon, Nov 30, 2020 at 03:21:37PM +, Andrey Zhizhikin wrote:
> Commit 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is
> unused") removed geenric_bl driver from the tree, together with
> corresponding config option.
>
> Remove BACKLIGHT_GENERIC config item from generic-64bit_defconfig.
>
> Fixes: 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is
> unused")
> Cc: Sam Ravnborg
> Signed-off-by: Andrey Zhizhikin
> ---
> arch/powerpc/configs/powernv_defconfig | 1 -
> 1 file changed, 1 deletion(-)
>
Reviewed-by: Krzysztof Kozlowski
Best regards,
Krzysztof
Re: [PATCH 4/5] parisc: configs: drop unused BACKLIGHT_GENERIC option
On Mon, Nov 30, 2020 at 03:21:36PM +, Andrey Zhizhikin wrote:
> Commit 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is
> unused") removed geenric_bl driver from the tree, together with
> corresponding config option.
>
> Remove BACKLIGHT_GENERIC config item from generic-64bit_defconfig.
>
> Fixes: 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is
> unused")
> Cc: Sam Ravnborg
> Signed-off-by: Andrey Zhizhikin
> ---
> arch/parisc/configs/generic-64bit_defconfig | 1 -
> 1 file changed, 1 deletion(-)
>
Reviewed-by: Krzysztof Kozlowski
Best regards,
Krzysztof
Re: [PATCH 3/5] MIPS: configs: drop unused BACKLIGHT_GENERIC option
On Mon, Nov 30, 2020 at 03:21:35PM +, Andrey Zhizhikin wrote:
> Commit 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is
> unused") removed geenric_bl driver from the tree, together with
> corresponding config option.
>
> Remove BACKLIGHT_GENERIC config item from all MIPS configurations.
>
> Fixes: 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is
> unused")
> Cc: Sam Ravnborg
> Signed-off-by: Andrey Zhizhikin
> ---
> arch/mips/configs/gcw0_defconfig | 1 -
> arch/mips/configs/gpr_defconfig | 1 -
> arch/mips/configs/lemote2f_defconfig | 1 -
> arch/mips/configs/loongson3_defconfig | 1 -
> arch/mips/configs/mtx1_defconfig | 1 -
> arch/mips/configs/rs90_defconfig | 1 -
> 6 files changed, 6 deletions(-)
>
Reviewed-by: Krzysztof Kozlowski
Best regards,
Krzysztof
Re: [PATCH 2/5] arm64: defconfig: drop unused BACKLIGHT_GENERIC option
On Mon, Nov 30, 2020 at 03:21:34PM +, Andrey Zhizhikin wrote:
> Commit 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is
> unused") removed geenric_bl driver from the tree, together with
> corresponding config option.
>
> Remove BACKLIGHT_GENERIC config item from arm64 configuration.
>
> Fixes: 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is
> unused")
> Cc: Sam Ravnborg
> Signed-off-by: Andrey Zhizhikin
> ---
> arch/arm64/configs/defconfig | 1 -
> 1 file changed, 1 deletion(-)
>
Reviewed-by: Krzysztof Kozlowski
The same trouble as with ARM patch - this should go directly via
arm-soc.
Best regards,
Krzysztof
Re: [PATCH 1/5] ARM: configs: drop unused BACKLIGHT_GENERIC option
On Mon, Nov 30, 2020 at 03:21:33PM +, Andrey Zhizhikin wrote:
> Commit 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is
> unused") removed geenric_bl driver from the tree, together with
> corresponding config option.
>
> Remove BACKLIGHT_GENERIC config item from all ARM configurations.
>
> Fixes: 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is
> unused")
> Cc: Sam Ravnborg
> Signed-off-by: Andrey Zhizhikin
> ---
> arch/arm/configs/at91_dt_defconfig| 1 -
> arch/arm/configs/cm_x300_defconfig| 1 -
> arch/arm/configs/colibri_pxa300_defconfig | 1 -
> arch/arm/configs/jornada720_defconfig | 1 -
> arch/arm/configs/magician_defconfig | 1 -
> arch/arm/configs/mini2440_defconfig | 1 -
> arch/arm/configs/omap2plus_defconfig | 1 -
> arch/arm/configs/pxa3xx_defconfig | 1 -
> arch/arm/configs/qcom_defconfig | 1 -
> arch/arm/configs/sama5_defconfig | 1 -
> arch/arm/configs/sunxi_defconfig | 1 -
> arch/arm/configs/tegra_defconfig | 1 -
> arch/arm/configs/u8500_defconfig | 1 -
> 13 files changed, 13 deletions(-)
You need to send it to arm-soc maintainers, otherwise no one might feel
responsible enough to pick it up.
Reviewed-by: Krzysztof Kozlowski
+CC Arnd and Olof,
Dear Arnd and Olof,
Maybe it is worth to add arm-soc entry to the MAINTAINERS file?
Otherwise how one could get your email address? Not mentioning the
secret-soc address. :)
Best regards,
Krzysztof
Re: [PATCH v2 net-next 4/4] net: nfc: s3fwrn5: Support a UART interface
On Mon, Nov 30, 2020 at 09:05:45PM +0900, Bongsu Jeon wrote:
> From: Bongsu Jeon
>
> Since S3FWRN82 NFC Chip, The UART interface can be used.
> S3FWRN82 uses NCI protocol and supports I2C and UART interface.
>
> Signed-off-by: Bongsu Jeon
> ---
>
> Changes in v2:
>- remove the kfree(phy) because of duplicated free.
>- use the phy_common blocks.
>- wrap lines properly.
>
> drivers/nfc/s3fwrn5/Kconfig | 12 +++
> drivers/nfc/s3fwrn5/Makefile | 2 +
> drivers/nfc/s3fwrn5/phy_common.c | 12 +++
> drivers/nfc/s3fwrn5/phy_common.h | 1 +
> drivers/nfc/s3fwrn5/uart.c | 197
> +++
> 5 files changed, 224 insertions(+)
> create mode 100644 drivers/nfc/s3fwrn5/uart.c
>
> diff --git a/drivers/nfc/s3fwrn5/Kconfig b/drivers/nfc/s3fwrn5/Kconfig
> index 3f8b6da..8a6b1a7 100644
> --- a/drivers/nfc/s3fwrn5/Kconfig
> +++ b/drivers/nfc/s3fwrn5/Kconfig
> @@ -20,3 +20,15 @@ config NFC_S3FWRN5_I2C
> To compile this driver as a module, choose m here. The module will
> be called s3fwrn5_i2c.ko.
> Say N if unsure.
> +
> +config NFC_S3FWRN82_UART
> +tristate "Samsung S3FWRN82 UART support"
> +depends on NFC_NCI && SERIAL_DEV_BUS
> +select NFC_S3FWRN5
> +help
> + This module adds support for a UART interface to the S3FWRN82 chip.
> + Select this if your platform is using the UART bus.
> +
> + To compile this driver as a module, choose m here. The module will
> + be called s3fwrn82_uart.ko.
> + Say N if unsure.
> diff --git a/drivers/nfc/s3fwrn5/Makefile b/drivers/nfc/s3fwrn5/Makefile
> index 6b6f52d..7da827a 100644
> --- a/drivers/nfc/s3fwrn5/Makefile
> +++ b/drivers/nfc/s3fwrn5/Makefile
> @@ -5,6 +5,8 @@
>
> s3fwrn5-objs = core.o firmware.o nci.o phy_common.o
> s3fwrn5_i2c-objs = i2c.o
> +s3fwrn82_uart-objs = uart.o
>
> obj-$(CONFIG_NFC_S3FWRN5) += s3fwrn5.o
> obj-$(CONFIG_NFC_S3FWRN5_I2C) += s3fwrn5_i2c.o
> +obj-$(CONFIG_NFC_S3FWRN82_UART) += s3fwrn82_uart.o
> diff --git a/drivers/nfc/s3fwrn5/phy_common.c
> b/drivers/nfc/s3fwrn5/phy_common.c
> index 5cad1f4..497b02b 100644
> --- a/drivers/nfc/s3fwrn5/phy_common.c
> +++ b/drivers/nfc/s3fwrn5/phy_common.c
> @@ -47,6 +47,18 @@ bool s3fwrn5_phy_power_ctrl(struct phy_common *phy, enum
> s3fwrn5_mode mode)
> }
> EXPORT_SYMBOL(s3fwrn5_phy_power_ctrl);
>
> +void s3fwrn5_phy_set_mode(void *phy_id, enum s3fwrn5_mode mode)
> +{
> + struct phy_common *phy = phy_id;
> +
> + mutex_lock(>mutex);
> +
> + s3fwrn5_phy_power_ctrl(phy, mode);
> +
> + mutex_unlock(>mutex);
> +}
> +EXPORT_SYMBOL(s3fwrn5_phy_set_mode);
> +
> enum s3fwrn5_mode s3fwrn5_phy_get_mode(void *phy_id)
> {
> struct phy_common *phy = phy_id;
> diff --git a/drivers/nfc/s3fwrn5/phy_common.h
> b/drivers/nfc/s3fwrn5/phy_common.h
> index b98531d..99749c9 100644
> --- a/drivers/nfc/s3fwrn5/phy_common.h
> +++ b/drivers/nfc/s3fwrn5/phy_common.h
> @@ -31,6 +31,7 @@ struct phy_common {
>
> void s3fwrn5_phy_set_wake(void *phy_id, bool wake);
> bool s3fwrn5_phy_power_ctrl(struct phy_common *phy, enum s3fwrn5_mode mode);
> +void s3fwrn5_phy_set_mode(void *phy_id, enum s3fwrn5_mode mode);
> enum s3fwrn5_mode s3fwrn5_phy_get_mode(void *phy_id);
>
> #endif /* __NFC_S3FWRN5_PHY_COMMON_H */
> diff --git a/drivers/nfc/s3fwrn5/uart.c b/drivers/nfc/s3fwrn5/uart.c
> new file mode 100644
> index 000..f5ac017
> --- /dev/null
> +++ b/drivers/nfc/s3fwrn5/uart.c
> @@ -0,0 +1,197 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * UART Link Layer for S3FWRN82 NCI based Driver
> + *
> + * Copyright (C) 2015 Samsung Electronics
> + * Robert Baldyga
> + * Copyright (C) 2020 Samsung Electronics
> + * Bongsu Jeon
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "phy_common.h"
> +
> +#define S3FWRN82_NCI_HEADER 3
> +#define S3FWRN82_NCI_IDX 2
> +#define NCI_SKB_BUFF_LEN 258
> +
> +struct s3fwrn82_uart_phy {
> + struct phy_common common;
> + struct serdev_device *ser_dev;
> + struct sk_buff *recv_skb;
> +};
> +
> +static int s3fwrn82_uart_write(void *phy_id, struct sk_buff *out)
> +{
> + struct s3fwrn82_uart_phy *phy = phy_id;
> + int err;
> +
> + err = serdev_device_write(phy->ser_dev,
> + out->data, out->len,
> + MAX_SCHEDULE_TIMEOUT);
> + if (err < 0)
> + return err;
> +
> + return 0;
> +}
> +
> +static const struct s3fwrn5_phy_ops uart_phy_ops = {
> + .set_wake = s3fwrn5_phy_set_wake,
> + .set_mode = s3fwrn5_phy_set_mode,
> + .get_mode = s3fwrn5_phy_get_mode,
> + .write = s3fwrn82_uart_write,
> +};
> +
> +static int s3fwrn82_uart_read(struct serdev_device *serdev,
> + const unsigned char *data,
> + size_t count)
> +{
> + struct s3fwrn82_uart_phy *phy =
Re: [PATCH v3 net-next 3/4] nfc: s3fwrn5: extract the common phy blocks
On Mon, Nov 30, 2020 at 09:03:49PM +0900, Bongsu Jeon wrote: > From: Bongsu Jeon > > Extract the common phy blocks to reuse it. > The UART module will use the common blocks. > > Signed-off-by: Bongsu Jeon > --- > Changes in v3: >- move the phy_common object to s3fwrn.ko to avoid duplication. >- include the header files to include everything which is used inside. >- wrap the lines. > > Changes in v2: >- remove the common function's definition in common header file. >- make the common phy_common.c file to define the common function. >- wrap the lines. >- change the Header guard. >- remove the unused common function. > > drivers/nfc/s3fwrn5/Makefile | 2 +- > drivers/nfc/s3fwrn5/i2c.c| 117 > +-- > drivers/nfc/s3fwrn5/phy_common.c | 63 + > drivers/nfc/s3fwrn5/phy_common.h | 36 > 4 files changed, 139 insertions(+), 79 deletions(-) > create mode 100644 drivers/nfc/s3fwrn5/phy_common.c > create mode 100644 drivers/nfc/s3fwrn5/phy_common.h I am sorry, but I am not going to review this. Please send properly a next version - v4 of entire patchset - while fixing issues pointed out in my other email (so use proper workflow). Best regards, Krzysztof
Re: [PATCH net-next 2/4] nfc: s3fwrn5: reduce the EN_WAIT_TIME
On Mon, Nov 30, 2020 at 09:02:30PM +0900, Bongsu Jeon wrote: > From: Bongsu Jeon > > The delay of 20ms is enough to enable and > wake up the Samsung's nfc chip. > > Signed-off-by: Bongsu Jeon > --- > drivers/nfc/s3fwrn5/i2c.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) It's really not easy to work with your way of sending the patches. I am sorry but you have to adjust your style to the style of reviewers and the entire community. 1. Again, you ignored/dropped my Ack. 2. I asked you to send all patches referencing each other, which you can achieve without any effort with git format-patch and send-email or with in-reply-to. Seriously, these tools work properly by default! You have to break them on purpose - so stop. Now, all your patches are scattered over my mailbox. They are all over mailing list: https://lore.kernel.org/lkml/?q=bongsu.jeon2%40gmail.com Browsing this patchset is uncomfortable. It's a pain. Please, work on your workflow. Get help in that - there are plenty of open-source contributors in Samsung. Ask them how to do it. If you cannot, read the mailing lists and see how others do it. Recent example, one of thousands: https://lore.kernel.org/linux-arm-kernel/20201130131047.2648960-1-dan...@0x0f.com/T/#m4a9ed644869b8018b8286a6b229012278141cb66 1. It comes with a cover letter, 2. All emails are properly linked with each other (scroll to the bottom). Best regards, Krzysztof
Re: [PATCH v2 net-next 1/4] dt-bindings: net: nfc: s3fwrn5: Support a UART interface
On Mon, Nov 30, 2020 at 08:55:42AM -0800, Jakub Kicinski wrote: > On Mon, 30 Nov 2020 21:00:27 +0900 Bongsu jeon wrote: > > From: Bongsu Jeon > > > > Since S3FWRN82 NFC Chip, The UART interface can be used. > > S3FWRN82 supports I2C and UART interface. > > > > Signed-off-by: Bongsu Jeon > > All patches in the series should have the same version. > > If the patch was not changes in the given repost you can add: > > v3: > - no change > > Or just not mention the version in the changelog. > > It's also best to provide a cover letter describing what the series > does as a whole for series with more than 2 patches. Beside that I received just 1/4 of v2. LKML has one as well: https://lore.kernel.org/lkml/1606737627-29485-1-git-send-email-bongsu.j...@samsung.com/ Where are the others? Best regards, Krzysztof
Re: [PATCH v10 01/19] dt-bindings: memory: tegra20: emc: Document opp-supported-hw property
On Mon, Nov 30, 2020 at 11:48:18AM +0200, Georgi Djakov wrote: > On 23.11.20 2:27, Dmitry Osipenko wrote: > > Document opp-supported-hw property, which is not strictly necessary to > > have on Tegra20, but it's very convenient to have because all other SoC > > core devices will use hardware versioning, and thus, it's good to maintain > > the consistency. > > Hi Dmitry, > > I believe Krzysztof is waiting for Ack on the binding before merging > this patch (and the rest), but unfortunately it was not sent to the > DT mailing list for review. Indeed I am still waiting for Rob's and Thierry's acks for this and the following patches. It has been just a week so I'll give it few more days. Best regards, Krzysztof
Re: [PATCH] ARM: omap2plus_defconfig: drop unused POWER_AVS option
On Mon, Nov 30, 2020 at 12:42:33PM +, Andrey Zhizhikin wrote:
> Commit 785b5bb41b0a ("PM: AVS: Drop the avs directory and the
> corresponding Kconfig") moved AVS code to SOC-specific folders, and
> removed corresponding Kconfig from drivers/power, leaving original
> POWER_AVS config option enabled in omap2plus_defconfig file.
>
> Remove the option, which has no references in the tree anymore.
>
> Fixes: 785b5bb41b0a ("PM: AVS: Drop the avs directory and the corresponding
> Kconfig")
> Cc: Nishanth Menon
> Cc: Ulf Hansson
> Signed-off-by: Andrey Zhizhikin
I thought you will squash it with multi_v7, but it's fine this way.
Reviewed-by: Krzysztof Kozlowski
Best regards,
Krzysztof
Re: [PATCH 2/2] ARM: multi_v7_defconfig: drop unused POWER_AVS option
On Mon, Nov 30, 2020 at 11:27:31AM +, Andrey Zhizhikin wrote:
> Commit 785b5bb41b0a ("PM: AVS: Drop the avs directory and the
> corresponding Kconfig") moved AVS code to SOC-specific folders, and
> removed corresponding Kconfig from drivers/power, leaving original
> POWER_AVS config option enabled in multi_v7_defconfig file.
>
> Remove the option, which has no references in the tree anymore.
>
> Fixes: 785b5bb41b0a ("PM: AVS: Drop the avs directory and the corresponding
> Kconfig")
> Cc: Nishanth Menon
> Cc: Ulf Hansson
> Signed-off-by: Andrey Zhizhikin
> ---
> arch/arm/configs/multi_v7_defconfig | 1 -
> 1 file changed, 1 deletion(-)
>
Reviewed-by: Krzysztof Kozlowski
Best regards,
Krzysztof
Re: [PATCH 2/2] ARM: multi_v7_defconfig: drop unused POWER_AVS option
On Mon, Nov 30, 2020 at 11:27:31AM +, Andrey Zhizhikin wrote:
> Commit 785b5bb41b0a ("PM: AVS: Drop the avs directory and the
> corresponding Kconfig") moved AVS code to SOC-specific folders, and
> removed corresponding Kconfig from drivers/power, leaving original
> POWER_AVS config option enabled in multi_v7_defconfig file.
>
> Remove the option, which has no references in the tree anymore.
>
> Fixes: 785b5bb41b0a ("PM: AVS: Drop the avs directory and the corresponding
> Kconfig")
> Cc: Nishanth Menon
> Cc: Ulf Hansson
> Signed-off-by: Andrey Zhizhikin
Thanks for the patch. You should also remove it from the omap2plus config.
Best regards,
Krzysztof
Re: [PATCH 1/2] arm64: defconfig: drop unused POWER_AVS option
On Mon, Nov 30, 2020 at 11:27:30AM +, Andrey Zhizhikin wrote:
> Commit 785b5bb41b0a ("PM: AVS: Drop the avs directory and the
> corresponding Kconfig") moved AVS code to SOC-specific folders, and
> removed corresponding Kconfig from drivers/power, leaving original
> POWER_AVS config option enabled in defconfig file.
>
> Remove the option, which has no references in the tree anymore.
>
> Fixes: 785b5bb41b0a ("PM: AVS: Drop the avs directory and the corresponding
> Kconfig")
> Cc: Nishanth Menon
> Cc: Ulf Hansson
> Signed-off-by: Andrey Zhizhikin
> ---
> arch/arm64/configs/defconfig | 1 -
> 1 file changed, 1 deletion(-)
>
Reviewed-by: Krzysztof Kozlowski
Best regards,
Krzysztof
Re: [PATCH v10 00/19] Introduce memory interconnect for NVIDIA Tegra SoCs
On Mon, Nov 30, 2020 at 05:44:39PM +0900, Chanwoo Choi wrote: > Hi Dmitry, > > The v5.10-rc6 was released from linus git tree. > Generally, I will send the pull-quest about devfreq to linux-pm.git maintainer > after releasing the v5.1-rc7 for the integration test on linux-pm.git. > > The icc patches in this patch have not yet merged. If these patches > are not merged before v5.10-rc7, Maybe, I'll apply the devfreq patches > for v5.12-rc1. None of the patches here are going to be merged to Linus' in the current cycle. They will only go to the next so if there is dependency, everything will be broken and non-bisectable. However no such dependencies or merging requirements were mention in the cover letter. Best regards, Krzysztof
Re: [PATCH v2 net-next 3/3] nfc: s3fwrn5: extract the common phy blocks
On Sun, Nov 29, 2020 at 06:55:10PM +0900, Bongsu Jeon wrote:
> On Sat, Nov 28, 2020 at 9:49 PM Krzysztof Kozlowski wrote:
> > This is not a proper wrapping. Wrapping happens on function arguments.
> >
> > > + if (!gpio_is_valid(phy->common.gpio_en))
> > > return -ENODEV;
> > > }
> > >
> > > - phy->gpio_fw_wake = of_get_named_gpio(np, "wake-gpios", 0);
> > > - if (!gpio_is_valid(phy->gpio_fw_wake)) {
> > > + phy->common.gpio_fw_wake = of_get_named_gpio(np, "wake-gpios", 0);
> > > + if (!gpio_is_valid(phy->common.gpio_fw_wake)) {
> > > /* Support also deprecated property */
> > > - phy->gpio_fw_wake = of_get_named_gpio(np,
> > > "s3fwrn5,fw-gpios", 0);
> > > - if (!gpio_is_valid(phy->gpio_fw_wake))
> > > + phy->common.gpio_fw_wake =
> > > + of_get_named_gpio(np, "s3fwrn5,fw-gpios",
> > > 0);
> > > + if (!gpio_is_valid(phy->common.gpio_fw_wake))
> >
> > The same.
> >
>
> Even though I wrapped this as below, Second line("s3fwrn5,fw-gpios" )
> was over 80 columns.
> Is it okay as below?
> phy->gpio_fw_wake =of_get_named_gpio(np,
>
> "s3fwrn5,fw-gpios",
> 0);
Your email client breaks indentation so I cannot answer. The wrapping - as
explained in CodingStyle - would look like:
phy->common.gpio_fw_wake = of_get_named_gpio(np,
"s3fwrn5,fw-gpios",
0);
It's not a problem if single argument exceeds 80 character.
>
>
> > > return -ENODEV;
> > > }
> > >
> > > @@ -226,8 +184,8 @@ static int s3fwrn5_i2c_probe(struct i2c_client
> > > *client,
> > > if (!phy)
> > > return -ENOMEM;
> > >
> > > - mutex_init(>mutex);
> > > - phy->mode = S3FWRN5_MODE_COLD;
> > > + mutex_init(>common.mutex);
> > > + phy->common.mode = S3FWRN5_MODE_COLD;
> > > phy->irq_skip = true;
> > >
> > > phy->i2c_dev = client;
> > > @@ -237,17 +195,19 @@ static int s3fwrn5_i2c_probe(struct i2c_client
> > > *client,
> > > if (ret < 0)
> > > return ret;
> > >
> > > - ret = devm_gpio_request_one(>i2c_dev->dev, phy->gpio_en,
> > > - GPIOF_OUT_INIT_HIGH, "s3fwrn5_en");
> > > + ret = devm_gpio_request_one(>i2c_dev->dev, phy->common.gpio_en,
> > > + GPIOF_OUT_INIT_HIGH, "s3fwrn5_en");
> > > if (ret < 0)
> > > return ret;
> > >
> > > - ret = devm_gpio_request_one(>i2c_dev->dev, phy->gpio_fw_wake,
> > > - GPIOF_OUT_INIT_LOW, "s3fwrn5_fw_wake");
> > > + ret = devm_gpio_request_one(>i2c_dev->dev,
> > > + phy->common.gpio_fw_wake,
> > > + GPIOF_OUT_INIT_LOW, "s3fwrn5_fw_wake");
> > > if (ret < 0)
> > > return ret;
> > >
> > > - ret = s3fwrn5_probe(>ndev, phy, >i2c_dev->dev,
> > > _phy_ops);
> > > + ret = s3fwrn5_probe(>common.ndev, phy, >i2c_dev->dev,
> > > + _phy_ops);
> > > if (ret < 0)
> > > return ret;
> > >
> > > @@ -255,7 +215,7 @@ static int s3fwrn5_i2c_probe(struct i2c_client
> > > *client,
> > > s3fwrn5_i2c_irq_thread_fn, IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
> > > S3FWRN5_I2C_DRIVER_NAME, phy);
> > > if (ret)
> > > - s3fwrn5_remove(phy->ndev);
> > > + s3fwrn5_remove(phy->common.ndev);
> > >
> > > return ret;
> > > }
> > > @@ -264,7 +224,7 @@ static int s3fwrn5_i2c_remove(struct i2c_client
> > > *client)
> > > {
> > > struct s3fwrn5_i2c_phy *phy = i2c_get_clientdata(client);
> > >
> > > - s3fwrn5_remove(phy->ndev);
> > > + s3fwrn5_remove(phy->common.ndev);
> > >
> > > return 0;
> > > }
> > > diff --
Re: [PATCH v2 4/5] memory: renesas-rpc-if: Make rpcif_enable/disable_rpm() as static inline
On Thu, Nov 26, 2020 at 07:11:45PM +, Lad Prabhakar wrote: > Define rpcif_enable_rpm() and rpcif_disable_rpm() as static > inline in the header instead of exporting them. > > Suggested-by: Pavel Machek > Signed-off-by: Lad Prabhakar > --- > drivers/memory/renesas-rpc-if.c | 13 - > include/memory/renesas-rpc-if.h | 13 +++-- > 2 files changed, 11 insertions(+), 15 deletions(-) Thanks, applied. Best regards, Krzysztof
Re: [PATCH v2 1/5] memory: renesas-rpc-if: Return correct value to the caller of rpcif_manual_xfer()
On Thu, Nov 26, 2020 at 07:11:42PM +, Lad Prabhakar wrote:
> In the error path of rpcif_manual_xfer() the value of ret is overwritten
> by value returned by reset_control_reset() function and thus returning
> incorrect value to the caller.
>
> This patch makes sure the correct value is returned to the caller of
> rpcif_manual_xfer() by dropping the overwrite of ret in error path.
> Also now we ignore the value returned by reset_control_reset() in the
> error path and instead print a error message when it fails.
>
> Fixes: ca7d8b980b67f ("memory: add Renesas RPC-IF driver")
> Reported-by: Pavel Machek
> Signed-off-by: Lad Prabhakar
> Cc: sta...@vger.kernel.org
> Reviewed-by: Sergei Shtylyov
> ---
> drivers/memory/renesas-rpc-if.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
Thanks, applied to mem-ctrl tree.
Best regards,
Krzysztof
Re: [PATCH v2 net-next 3/3] nfc: s3fwrn5: extract the common phy blocks
On Fri, Nov 27, 2020 at 08:22:18PM +0900, bongsu.je...@gmail.com wrote:
> From: Bongsu Jeon
>
> Extract the common phy blocks to reuse it.
> The UART module will use the common blocks.
>
> Signed-off-by: Bongsu Jeon
> ---
> Changes in v2:
> - remove the common function's definition in common header file.
> - make the common phy_common.c file to define the common function.
> - wrap the lines.
> - change the Header guard.
> - remove the unused common function.
>
> drivers/nfc/s3fwrn5/Makefile | 2 +-
> drivers/nfc/s3fwrn5/i2c.c| 114
> +--
> drivers/nfc/s3fwrn5/phy_common.c | 60 +
> drivers/nfc/s3fwrn5/phy_common.h | 31 +++
> 4 files changed, 129 insertions(+), 78 deletions(-)
> create mode 100644 drivers/nfc/s3fwrn5/phy_common.c
> create mode 100644 drivers/nfc/s3fwrn5/phy_common.h
>
> diff --git a/drivers/nfc/s3fwrn5/Makefile b/drivers/nfc/s3fwrn5/Makefile
> index d0ffa35..a5279c6 100644
> --- a/drivers/nfc/s3fwrn5/Makefile
> +++ b/drivers/nfc/s3fwrn5/Makefile
> @@ -4,7 +4,7 @@
> #
>
> s3fwrn5-objs = core.o firmware.o nci.o
> -s3fwrn5_i2c-objs = i2c.o
> +s3fwrn5_i2c-objs = i2c.o phy_common.o
Thanks for the changes.
Shouldn't this be part of s3fwrn5.ko? Otherwise you would duplicate the
objects in two modules.
>
> obj-$(CONFIG_NFC_S3FWRN5) += s3fwrn5.o
> obj-$(CONFIG_NFC_S3FWRN5_I2C) += s3fwrn5_i2c.o
> diff --git a/drivers/nfc/s3fwrn5/i2c.c b/drivers/nfc/s3fwrn5/i2c.c
> index 9a64eea..207f970 100644
> --- a/drivers/nfc/s3fwrn5/i2c.c
> +++ b/drivers/nfc/s3fwrn5/i2c.c
> @@ -16,74 +16,30 @@
> #include
>
> #include "s3fwrn5.h"
> +#include "phy_common.h"
>
> #define S3FWRN5_I2C_DRIVER_NAME "s3fwrn5_i2c"
>
> -#define S3FWRN5_EN_WAIT_TIME 20
> -
> struct s3fwrn5_i2c_phy {
> + struct phy_common common;
> struct i2c_client *i2c_dev;
> - struct nci_dev *ndev;
> -
> - int gpio_en;
> - int gpio_fw_wake;
> -
> - struct mutex mutex;
>
> - enum s3fwrn5_mode mode;
> unsigned int irq_skip:1;
> };
>
> -static void s3fwrn5_i2c_set_wake(void *phy_id, bool wake)
> -{
> - struct s3fwrn5_i2c_phy *phy = phy_id;
> -
> - mutex_lock(>mutex);
> - gpio_set_value(phy->gpio_fw_wake, wake);
> - msleep(S3FWRN5_EN_WAIT_TIME);
> - mutex_unlock(>mutex);
> -}
> -
> static void s3fwrn5_i2c_set_mode(void *phy_id, enum s3fwrn5_mode mode)
> {
> struct s3fwrn5_i2c_phy *phy = phy_id;
>
> - mutex_lock(>mutex);
> + mutex_lock(>common.mutex);
>
> - if (phy->mode == mode)
> + if (s3fwrn5_phy_power_ctrl(>common, mode) == false)
> goto out;
>
> - phy->mode = mode;
> -
> - gpio_set_value(phy->gpio_en, 1);
> - gpio_set_value(phy->gpio_fw_wake, 0);
> - if (mode == S3FWRN5_MODE_FW)
> - gpio_set_value(phy->gpio_fw_wake, 1);
> -
> - if (mode != S3FWRN5_MODE_COLD) {
> - msleep(S3FWRN5_EN_WAIT_TIME);
> - gpio_set_value(phy->gpio_en, 0);
> - msleep(S3FWRN5_EN_WAIT_TIME);
> - }
> -
> phy->irq_skip = true;
>
> out:
> - mutex_unlock(>mutex);
> -}
> -
> -static enum s3fwrn5_mode s3fwrn5_i2c_get_mode(void *phy_id)
> -{
> - struct s3fwrn5_i2c_phy *phy = phy_id;
> - enum s3fwrn5_mode mode;
> -
> - mutex_lock(>mutex);
> -
> - mode = phy->mode;
> -
> - mutex_unlock(>mutex);
> -
> - return mode;
> + mutex_unlock(>common.mutex);
> }
>
> static int s3fwrn5_i2c_write(void *phy_id, struct sk_buff *skb)
> @@ -91,7 +47,7 @@ static int s3fwrn5_i2c_write(void *phy_id, struct sk_buff
> *skb)
> struct s3fwrn5_i2c_phy *phy = phy_id;
> int ret;
>
> - mutex_lock(>mutex);
> + mutex_lock(>common.mutex);
>
> phy->irq_skip = false;
>
> @@ -102,7 +58,7 @@ static int s3fwrn5_i2c_write(void *phy_id, struct sk_buff
> *skb)
> ret = i2c_master_send(phy->i2c_dev, skb->data, skb->len);
> }
>
> - mutex_unlock(>mutex);
> + mutex_unlock(>common.mutex);
>
> if (ret < 0)
> return ret;
> @@ -114,9 +70,9 @@ static int s3fwrn5_i2c_write(void *phy_id, struct sk_buff
> *skb)
> }
>
> static const struct s3fwrn5_phy_ops i2c_phy_ops = {
> - .set_wake = s3fwrn5_i2c_set_wake,
> + .set_wake = s3fwrn5_phy_set_wake,
> .set_mode = s3fwrn5_i2c_set_mode,
> - .get_mode = s3fwrn5_i2c_get_mode,
> + .get_mode = s3fwrn5_phy_get_mode,
> .write = s3fwrn5_i2c_write,
> };
>
> @@ -128,7 +84,7 @@ static int s3fwrn5_i2c_read(struct s3fwrn5_i2c_phy *phy)
> char hdr[4];
> int ret;
>
> - hdr_size = (phy->mode == S3FWRN5_MODE_NCI) ?
> + hdr_size = (phy->common.mode == S3FWRN5_MODE_NCI) ?
> NCI_CTRL_HDR_SIZE : S3FWRN5_FW_HDR_SIZE;
> ret = i2c_master_recv(phy->i2c_dev, hdr, hdr_size);
> if (ret < 0)
> @@ -137,7 +93,7 @@ static int s3fwrn5_i2c_read(struct s3fwrn5_i2c_phy *phy)
> if (ret < hdr_size)
> return
Re: [PATCH v2 3/5] memory: renesas-rpc-if: Fix a reference leak in rpcif_probe()
On Fri, Nov 27, 2020 at 11:41:14PM +0100, Pavel Machek wrote:
> On Thu 2020-11-26 19:11:44, Lad Prabhakar wrote:
> > Release the node reference by calling of_node_put(flash) in the probe.
> >
> > Fixes: ca7d8b980b67f ("memory: add Renesas RPC-IF driver")
> > Reported-by: Pavel Machek
> > Signed-off-by: Lad Prabhakar
> > Cc: sta...@vger.kernel.org
> > Reviewed-by: Sergei Shtylyov
>
> Reviewed-by: Pavel Machek (CIP)<
This breaks b4. Corrected and applied.
Best regards,
Krzysztof
Re: [PATCH v2 2/5] memory: renesas-rpc-if: Fix unbalanced pm_runtime_enable in rpcif_{enable,disable}_rpm
On Thu, Nov 26, 2020 at 07:11:43PM +, Lad Prabhakar wrote:
> rpcif_enable_rpm calls pm_runtime_enable, so rpcif_disable_rpm needs to
> call pm_runtime_disable and not pm_runtime_put_sync.
>
> Fixes: ca7d8b980b67f ("memory: add Renesas RPC-IF driver")
> Reported-by: Reported-by: Geert Uytterhoeven
> Signed-off-by: Lad Prabhakar
> Cc: sta...@vger.kernel.org
> ---
> drivers/memory/renesas-rpc-if.c | 2 +-
Thanks, applied with corrected Reported-by.
Best regards,
Krzysztof
Re: [PATCH v2 5/5] memory: renesas-rpc-if: Export symbols as GPL
On Thu, Nov 26, 2020 at 07:11:46PM +, Lad Prabhakar wrote: > Renesas RPC-IF driver is licensed under GPL2.0, to be in sync export the > symbols as GPL. It's not a valid reason to export them as GPL. Entire Linux source code is licensed as GPL-2.0, so are you going to change all EXPORT_SYMBOL to GPL? Please describe it better. Usually the symbols are exported as GPL if they are considered tightly coupled with the kernel code. So tightly that basically it is not a interface anymore but part of kernel internals and therefore any usage of it is a derivative work of Linux kernel. If this is the case here, please describe in commit msg why these match this criteria. Best regards, Krzysztof
Re: [PATCH net-next 3/3] nfc: s3fwrn5: extract the common phy blocks
On Fri, Nov 27, 2020 at 08:09:24AM +0900, Bongsu Jeon wrote:
> On Fri, Nov 27, 2020 at 2:13 AM Krzysztof Kozlowski wrote:
> >
> > On Fri, Nov 27, 2020 at 12:33:39AM +0900, bongsu.je...@gmail.com wrote:
> > > From: Bongsu Jeon
> > >
> > > Extract the common phy blocks to reuse it.
> > > The UART module will use the common blocks.
> >
> >
> > Hi,
> >
> > Thanks for the patch. Few comments below.
> >
> > > Signed-off-by: Bongsu Jeon
> > > ---
> > > drivers/nfc/s3fwrn5/i2c.c| 111
> > > ---
> > > drivers/nfc/s3fwrn5/phy_common.h | 86 ++
> > > 2 files changed, 119 insertions(+), 78 deletions(-)
> > > create mode 100644 drivers/nfc/s3fwrn5/phy_common.h
> > >
> > > diff --git a/drivers/nfc/s3fwrn5/i2c.c b/drivers/nfc/s3fwrn5/i2c.c
> > > index 9a64eea..cd1b2a7 100644
> > > --- a/drivers/nfc/s3fwrn5/i2c.c
> > > +++ b/drivers/nfc/s3fwrn5/i2c.c
> > > @@ -15,75 +15,30 @@
> > >
> > > #include
> > >
> > > -#include "s3fwrn5.h"
> > > +#include "phy_common.h"
> > >
> > > #define S3FWRN5_I2C_DRIVER_NAME "s3fwrn5_i2c"
> > >
> > > -#define S3FWRN5_EN_WAIT_TIME 20
> > > -
> > > struct s3fwrn5_i2c_phy {
> > > + struct phy_common common;
> > > struct i2c_client *i2c_dev;
> > > - struct nci_dev *ndev;
> > > -
> > > - int gpio_en;
> > > - int gpio_fw_wake;
> > > -
> > > - struct mutex mutex;
> > >
> > > - enum s3fwrn5_mode mode;
> > > unsigned int irq_skip:1;
> > > };
> > >
> > > -static void s3fwrn5_i2c_set_wake(void *phy_id, bool wake)
> > > -{
> > > - struct s3fwrn5_i2c_phy *phy = phy_id;
> > > -
> > > - mutex_lock(>mutex);
> > > - gpio_set_value(phy->gpio_fw_wake, wake);
> > > - msleep(S3FWRN5_EN_WAIT_TIME);
> > > - mutex_unlock(>mutex);
> > > -}
> > > -
> > > static void s3fwrn5_i2c_set_mode(void *phy_id, enum s3fwrn5_mode mode)
> > > {
> > > struct s3fwrn5_i2c_phy *phy = phy_id;
> > >
> > > - mutex_lock(>mutex);
> > > + mutex_lock(>common.mutex);
> > >
> > > - if (phy->mode == mode)
> > > + if (s3fwrn5_phy_power_ctrl(>common, mode) == false)
> > > goto out;
> > >
> > > - phy->mode = mode;
> > > -
> > > - gpio_set_value(phy->gpio_en, 1);
> > > - gpio_set_value(phy->gpio_fw_wake, 0);
> > > - if (mode == S3FWRN5_MODE_FW)
> > > - gpio_set_value(phy->gpio_fw_wake, 1);
> > > -
> > > - if (mode != S3FWRN5_MODE_COLD) {
> > > - msleep(S3FWRN5_EN_WAIT_TIME);
> > > - gpio_set_value(phy->gpio_en, 0);
> > > - msleep(S3FWRN5_EN_WAIT_TIME);
> > > - }
> > > -
> > > phy->irq_skip = true;
> > >
> > > out:
> > > - mutex_unlock(>mutex);
> > > -}
> > > -
> > > -static enum s3fwrn5_mode s3fwrn5_i2c_get_mode(void *phy_id)
> > > -{
> > > - struct s3fwrn5_i2c_phy *phy = phy_id;
> > > - enum s3fwrn5_mode mode;
> > > -
> > > - mutex_lock(>mutex);
> > > -
> > > - mode = phy->mode;
> > > -
> > > - mutex_unlock(>mutex);
> > > -
> > > - return mode;
> > > + mutex_unlock(>common.mutex);
> > > }
> > >
> > > static int s3fwrn5_i2c_write(void *phy_id, struct sk_buff *skb)
> > > @@ -91,7 +46,7 @@ static int s3fwrn5_i2c_write(void *phy_id, struct
> > > sk_buff *skb)
> > > struct s3fwrn5_i2c_phy *phy = phy_id;
> > > int ret;
> > >
> > > - mutex_lock(>mutex);
> > > + mutex_lock(>common.mutex);
> > >
> > > phy->irq_skip = false;
> > >
> > > @@ -102,7 +57,7 @@ static int s3fwrn5_i2c_write(void *phy_id, struct
> > > sk_buff *skb)
> > > ret = i2c_master_send(phy->i2c_dev, skb->data, skb->len);
> > > }
> > >
> > > - mutex_unlock(>mutex);
> > > + mutex_unlock(>common.mutex);
> > >
> > > if (ret < 0)
> > > retu
Re: [PATCH v1 6/8] arm64: defconfig: Enable CONFIG_VCNL4000
On Fri, 27 Nov 2020 at 15:23, Guido Günther wrote: > > This is the Librem 5's proximity sensor. Just squash all of them. Enabling option by option is too much. Best regards, Krzysztof
Re: [PATCH] hwrng: exynos - fix reference leak in exynos_trng_probe
On Fri, Nov 27, 2020 at 05:44:46PM +0800, Qinglang Miao wrote:
> pm_runtime_get_sync will increment pm usage counter even it
> failed. Forgetting to putting operation will result in a
> reference leak here.
>
> A new function pm_runtime_resume_and_get is introduced in
> [0] to keep usage counter balanced. So We fix the reference
> leak by replacing it with new funtion.
>
> [0] dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with
> usage counter")
Do not put such dependencies into the commit message - it does not bring
useful information to the history. Store it under '---' separator.
>
> Fixes: 6cd225cc5d8a ("hwrng: exynos - add Samsung Exynos True RNG driver")
> Reported-by: Hulk Robot
> Signed-off-by: Qinglang Miao
> ---
> drivers/char/hw_random/exynos-trng.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/char/hw_random/exynos-trng.c
> b/drivers/char/hw_random/exynos-trng.c
> index 8e1fe3f8d..666246bc8 100644
> --- a/drivers/char/hw_random/exynos-trng.c
> +++ b/drivers/char/hw_random/exynos-trng.c
> @@ -132,7 +132,7 @@ static int exynos_trng_probe(struct platform_device *pdev)
> return PTR_ERR(trng->mem);
>
> pm_runtime_enable(>dev);
> - ret = pm_runtime_get_sync(>dev);
> + ret = pm_runtime_resume_and_get(>dev);
This cannot be applied. Fix it by replacing err_clock label with this
one.
Best regards,
Krzysztof
Re: [PATCH 01/16] mfd: bcm590xx: drop of_match_ptr from of_device_id table
On Fri, 27 Nov 2020 at 09:06, Lee Jones wrote: > > On Fri, 20 Nov 2020, Krzysztof Kozlowski wrote: > > > The driver can match only via the DT table so the table should be always > > used and the of_match_ptr does not have any sense (this also allows ACPI > > matching via PRP0001, even though it is not relevant here). This fixes > > compile warning (!CONFIG_OF on x86_64): > > > > drivers/mfd/bcm590xx.c:95:34: warning: ‘bcm590xx_of_match’ defined but > > not used [-Wunused-const-variable=] > > > > Signed-off-by: Krzysztof Kozlowski > > --- > > drivers/mfd/bcm590xx.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > Couple of small points: > > - Patch-sets, particularly large ones should have a cover letter. > - Subject lines follow English grammar conventions and should start > with an uppercase character. Thanks for fixing this. I am not a native English speaker and I make mistakes. Either it is a grammar or convention mistake, I would appreciate it if you point them out, so I could learn from them. About the "start with an uppercase character", I actually on purpose changed my approach some time ago after seeing more senior kernel developers using this method (see commits from Linus Torvalds, Andrew Morton). After the subsystem prefix "mfd: " they start with lowercase. If you still insist that commit titles in MFD subsystem should use capital letter after the prefix, I will try to remember and follow this approach when sending patches to you. Best regards, Krzysztof
Re: [PATCH v7 17/47] dt-bindings: memory: tegra20: Add memory client IDs
On Thu, Nov 26, 2020 at 07:02:55PM +0100, Thierry Reding wrote: > On Thu, Nov 26, 2020 at 06:39:22PM +0100, Krzysztof Kozlowski wrote: > > On Thu, Nov 26, 2020 at 06:26:05PM +0100, Thierry Reding wrote: > > > On Wed, Nov 04, 2020 at 07:48:53PM +0300, Dmitry Osipenko wrote: > > > > Each memory client has unique hardware ID, add these IDs. > > > > > > > > Acked-by: Rob Herring > > > > Signed-off-by: Dmitry Osipenko > > > > --- > > > > include/dt-bindings/memory/tegra20-mc.h | 53 + > > > > 1 file changed, 53 insertions(+) > > > > > > Is there any chance you could drop these dt-bindings include patches > > > (17, 18 and 19) so that I can pick them up into the Tegra tree? The > > > device tree changes that I was going to pick up depend on this and > > > fail to build if applied as-is. > > > > > > I was looking at your linux-mem-ctrl tree and had initially thought I > > > could just pull in one of the branches to get these dependencies, but it > > > looks like the dt-bindings patches are on the for-v5.11/tegra-mc branch, > > > which the ARM SoC maintainers wouldn't like to see me pull in for a > > > dependency on device tree changes. > > > > Partially you answered here. :) Since you should not pull my branch into > > a DT branch, you also should not put these include/dt-bindings patches > > there. SoC guys will complain about this as well. > > > > These patches are also needed for the driver, so if you take them, I > > would need them back in a pull request. SoC folks could spot it as well > > and point that such merge should not happen. > > > > > If this is all fixed at this point, I'll just have to push back the > > > device tree changes to v5.12, or perhaps see if the ARM SoC maintainers > > > are willing to take a late pull request that's based on v5.11-rc1. > > > > Yeah, that's a known problem. I asked about this Arnd and Olof in the > > past and got reply with two solutions: > > 1. Apply current version of patch without defines, just hard-coded > >numbers. After merging to Linus, replace the numbers with defines. > > > > 2. Wait with DTS till dependencies reach Linus. > > What I've done occasionally in the past was to put these kinds of > patches into a separate "dt-bindings" branch that I could use to resolve > dependencies from device tree files. The ARM SoC maintainers never had > any issues with that approach. > > I guess this is a bit of a special case, because the DT includes are > ultimately really a part of the device tree, so mixing them both isn't > problematic. Indeed, that way could work... and no one would spot it. :) Many times these headers were for clock symbols so if they go via SoC/DT tree, merge back to clock tree could be accepted. Best regards, Krzysztof
Re: [PATCH v7 18/47] dt-bindings: memory: tegra30: Add memory client IDs
On Wed, Nov 04, 2020 at 07:48:54PM +0300, Dmitry Osipenko wrote: > Each memory client has unique hardware ID, add these IDs. > > Acked-by: Rob Herring > Signed-off-by: Dmitry Osipenko > --- > include/dt-bindings/memory/tegra30-mc.h | 67 + > 1 file changed, 67 insertions(+) > Acked-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH v7 19/47] dt-bindings: memory: tegra124: Add memory client IDs
On Wed, Nov 04, 2020 at 07:48:55PM +0300, Dmitry Osipenko wrote: > Each memory client has unique hardware ID, add these IDs. > > Reviewed-by: Rob Herring > Signed-off-by: Dmitry Osipenko > --- > include/dt-bindings/memory/tegra124-mc.h | 68 > 1 file changed, 68 insertions(+) > Acked-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH v7 17/47] dt-bindings: memory: tegra20: Add memory client IDs
On Thu, Nov 26, 2020 at 06:45:51PM +0100, Krzysztof Kozlowski wrote: > On Thu, 26 Nov 2020 at 18:39, Krzysztof Kozlowski wrote: > > > > On Thu, Nov 26, 2020 at 06:26:05PM +0100, Thierry Reding wrote: > > > On Wed, Nov 04, 2020 at 07:48:53PM +0300, Dmitry Osipenko wrote: > > > > Each memory client has unique hardware ID, add these IDs. > > > > > > > > Acked-by: Rob Herring > > > > Signed-off-by: Dmitry Osipenko > > > > --- > > > > include/dt-bindings/memory/tegra20-mc.h | 53 + > > > > 1 file changed, 53 insertions(+) > > > > > > Is there any chance you could drop these dt-bindings include patches > > > (17, 18 and 19) so that I can pick them up into the Tegra tree? The > > > device tree changes that I was going to pick up depend on this and > > > fail to build if applied as-is. > > > > > > I was looking at your linux-mem-ctrl tree and had initially thought I > > > could just pull in one of the branches to get these dependencies, but it > > > looks like the dt-bindings patches are on the for-v5.11/tegra-mc branch, > > > which the ARM SoC maintainers wouldn't like to see me pull in for a > > > dependency on device tree changes. > > > > Partially you answered here. :) Since you should not pull my branch into > > a DT branch, you also should not put these include/dt-bindings patches > > there. SoC guys will complain about this as well. > > > > These patches are also needed for the driver, so if you take them, I > > would need them back in a pull request. SoC folks could spot it as well > > and point that such merge should not happen. > > It seems I was wrong - these patches are not needed for the driver > code. Neither in applied parts nor in upcoming Dmitry's work. In such > case I could rework my branches and send a new pull request. The > patches would stay only in your tree. Acked-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [GIT PULL 2/2] memory: tegra for v5.11
On Wed, Nov 25, 2020 at 07:45:29PM +0100, Krzysztof Kozlowski wrote: > Hi, > > The Tegra memory controllers work was big enough to get its own branch and > tag. > It also includes few SoC and clock patches, which I shared externally via a > stable tag to devfreq tree. > > The work is not yet finished, so more patches from Dmitry will be coming. > However I want to flush my queue now. Hi Arnd and Olof, Please ignore this pull. Only this one. The 1/2 is good. Thierry pointed out that include/dt-bindings patches are not used by drivers but are a requisite for DTS patches. I'll send a fixed pull request for 2/2. Best regards, Krzysztof > > Best regards, > Krzysztof > > > The following changes since commit 3650b228f83adda7e5ee532e2b90429c03f7b9ec: > > Linux 5.10-rc1 (2020-10-25 15:14:11 -0700) > > are available in the Git repository at: > > https://git.kernel.org/pub/scm/linux/kernel/git/krzk/linux-mem-ctrl.git > tags/memory-controller-drv-tegra-5.11 > > for you to fetch changes up to e45b57df4b9e9f8b5df7553a9a001acd9cae1b5d: > > memory: tegra30-emc: Remove unnecessary of_node_put in tegra_emc_probe > (2020-11-23 17:28:48 +0100) > > > Memory controller drivers for v5.11 - Tegra SoC > > There is a bigger work from Dmitry Osipenko around Tegra SoC memory > controller drivers, mostly towards adding interconnect support and > integration with devfreq. This work touches all Tegra memory controller > drivers and also few other SoC-related parts. It's not yet finished but > the intermediate stage seems ready to merge. > > Beside that Tegra 210 memory controller got few fixes and received new > swgroups (work of Nicolin Chen). > > > Dmitry Osipenko (35): > dt-bindings: memory: tegra20: emc: Correct registers range in example > clk: tegra: Export Tegra20 EMC kernel symbols > soc/tegra: fuse: Export tegra_read_ram_code() > soc/tegra: fuse: Add stub for tegra_sku_info > dt-bindings: memory: tegra20: emc: Document nvidia, memory-controller > property > dt-bindings: memory: tegra20: mc: Document new interconnect property > dt-bindings: memory: tegra20: emc: Document new interconnect property > dt-bindings: memory: tegra20: emc: Document OPP table and voltage > regulator > dt-bindings: memory: tegra30: mc: Document new interconnect property > dt-bindings: memory: tegra30: emc: Document new interconnect property > dt-bindings: memory: tegra30: emc: Document OPP table and voltage > regulator > dt-bindings: memory: tegra124: mc: Document new interconnect property > dt-bindings: memory: tegra124: emc: Document new interconnect property > dt-bindings: memory: tegra124: emc: Document OPP table and voltage > regulator > dt-bindings: tegra30-actmon: Document OPP and interconnect properties > dt-bindings: host1x: Document new interconnect properties > dt-bindings: memory: tegra20: Add memory client IDs > dt-bindings: memory: tegra30: Add memory client IDs > dt-bindings: memory: tegra124: Add memory client IDs > memory: tegra: Add and use devm_tegra_memory_controller_get() > memory: tegra: Use devm_platform_ioremap_resource() > memory: tegra: Remove superfluous error messages around > platform_get_irq() > memory: tegra: Add missing latency allowness entry for Page Table Cache > memory: tegra-mc: Add interconnect framework > memory: tegra20-emc: Make driver modular > memory: tegra20-emc: Continue probing if timings are missing in > device-tree > memory: tegra20: Support interconnect framework > memory: tegra: Correct stub of devm_tegra_memory_controller_get() > memory: tegra20-emc: Use dev_pm_opp_set_clkname() > memory: tegra20-emc: Factor out clk initialization > memory: tegra20-emc: Remove IRQ number from error message > memory: tegra20-emc: Add devfreq support > memory: tegra30: Add FIFO sizes to memory clients > memory: tegra30-emc: Make driver modular > memory: tegra30-emc: Continue probing if timings are missing in > device-tree > > Krzysztof Kozlowski (1): > Merge branch 'for-v5.11/tegra-soc-clk' into HEAD > > Nathan Chancellor (1): > memory: tegra30-emc: Remove unnecessary of_node_put in tegra_emc_probe > > Nicolin Chen (5): > memory: tegra: Correct la.reg address of seswr > memory: tegra: Correct tegra210_mc_clients def values > memory: tegra: Sort tegra210_swgroups by reg address > dt-bindings: memory: tegra: Add missing swgroups &g
Re: [PATCH v7 17/47] dt-bindings: memory: tegra20: Add memory client IDs
On Thu, 26 Nov 2020 at 18:39, Krzysztof Kozlowski wrote: > > On Thu, Nov 26, 2020 at 06:26:05PM +0100, Thierry Reding wrote: > > On Wed, Nov 04, 2020 at 07:48:53PM +0300, Dmitry Osipenko wrote: > > > Each memory client has unique hardware ID, add these IDs. > > > > > > Acked-by: Rob Herring > > > Signed-off-by: Dmitry Osipenko > > > --- > > > include/dt-bindings/memory/tegra20-mc.h | 53 + > > > 1 file changed, 53 insertions(+) > > > > Is there any chance you could drop these dt-bindings include patches > > (17, 18 and 19) so that I can pick them up into the Tegra tree? The > > device tree changes that I was going to pick up depend on this and > > fail to build if applied as-is. > > > > I was looking at your linux-mem-ctrl tree and had initially thought I > > could just pull in one of the branches to get these dependencies, but it > > looks like the dt-bindings patches are on the for-v5.11/tegra-mc branch, > > which the ARM SoC maintainers wouldn't like to see me pull in for a > > dependency on device tree changes. > > Partially you answered here. :) Since you should not pull my branch into > a DT branch, you also should not put these include/dt-bindings patches > there. SoC guys will complain about this as well. > > These patches are also needed for the driver, so if you take them, I > would need them back in a pull request. SoC folks could spot it as well > and point that such merge should not happen. It seems I was wrong - these patches are not needed for the driver code. Neither in applied parts nor in upcoming Dmitry's work. In such case I could rework my branches and send a new pull request. The patches would stay only in your tree. Best regards, Krzysztof
Re: [PATCH v7 17/47] dt-bindings: memory: tegra20: Add memory client IDs
On Thu, Nov 26, 2020 at 06:26:05PM +0100, Thierry Reding wrote: > On Wed, Nov 04, 2020 at 07:48:53PM +0300, Dmitry Osipenko wrote: > > Each memory client has unique hardware ID, add these IDs. > > > > Acked-by: Rob Herring > > Signed-off-by: Dmitry Osipenko > > --- > > include/dt-bindings/memory/tegra20-mc.h | 53 + > > 1 file changed, 53 insertions(+) > > Is there any chance you could drop these dt-bindings include patches > (17, 18 and 19) so that I can pick them up into the Tegra tree? The > device tree changes that I was going to pick up depend on this and > fail to build if applied as-is. > > I was looking at your linux-mem-ctrl tree and had initially thought I > could just pull in one of the branches to get these dependencies, but it > looks like the dt-bindings patches are on the for-v5.11/tegra-mc branch, > which the ARM SoC maintainers wouldn't like to see me pull in for a > dependency on device tree changes. Partially you answered here. :) Since you should not pull my branch into a DT branch, you also should not put these include/dt-bindings patches there. SoC guys will complain about this as well. These patches are also needed for the driver, so if you take them, I would need them back in a pull request. SoC folks could spot it as well and point that such merge should not happen. > If this is all fixed at this point, I'll just have to push back the > device tree changes to v5.12, or perhaps see if the ARM SoC maintainers > are willing to take a late pull request that's based on v5.11-rc1. Yeah, that's a known problem. I asked about this Arnd and Olof in the past and got reply with two solutions: 1. Apply current version of patch without defines, just hard-coded numbers. After merging to Linus, replace the numbers with defines. 2. Wait with DTS till dependencies reach Linus. Best regards, Krzysztof
Re: [PATCH net-next 3/3] nfc: s3fwrn5: extract the common phy blocks
On Fri, Nov 27, 2020 at 12:33:39AM +0900, bongsu.je...@gmail.com wrote:
> From: Bongsu Jeon
>
> Extract the common phy blocks to reuse it.
> The UART module will use the common blocks.
Hi,
Thanks for the patch. Few comments below.
> Signed-off-by: Bongsu Jeon
> ---
> drivers/nfc/s3fwrn5/i2c.c| 111
> ---
> drivers/nfc/s3fwrn5/phy_common.h | 86 ++
> 2 files changed, 119 insertions(+), 78 deletions(-)
> create mode 100644 drivers/nfc/s3fwrn5/phy_common.h
>
> diff --git a/drivers/nfc/s3fwrn5/i2c.c b/drivers/nfc/s3fwrn5/i2c.c
> index 9a64eea..cd1b2a7 100644
> --- a/drivers/nfc/s3fwrn5/i2c.c
> +++ b/drivers/nfc/s3fwrn5/i2c.c
> @@ -15,75 +15,30 @@
>
> #include
>
> -#include "s3fwrn5.h"
> +#include "phy_common.h"
>
> #define S3FWRN5_I2C_DRIVER_NAME "s3fwrn5_i2c"
>
> -#define S3FWRN5_EN_WAIT_TIME 20
> -
> struct s3fwrn5_i2c_phy {
> + struct phy_common common;
> struct i2c_client *i2c_dev;
> - struct nci_dev *ndev;
> -
> - int gpio_en;
> - int gpio_fw_wake;
> -
> - struct mutex mutex;
>
> - enum s3fwrn5_mode mode;
> unsigned int irq_skip:1;
> };
>
> -static void s3fwrn5_i2c_set_wake(void *phy_id, bool wake)
> -{
> - struct s3fwrn5_i2c_phy *phy = phy_id;
> -
> - mutex_lock(>mutex);
> - gpio_set_value(phy->gpio_fw_wake, wake);
> - msleep(S3FWRN5_EN_WAIT_TIME);
> - mutex_unlock(>mutex);
> -}
> -
> static void s3fwrn5_i2c_set_mode(void *phy_id, enum s3fwrn5_mode mode)
> {
> struct s3fwrn5_i2c_phy *phy = phy_id;
>
> - mutex_lock(>mutex);
> + mutex_lock(>common.mutex);
>
> - if (phy->mode == mode)
> + if (s3fwrn5_phy_power_ctrl(>common, mode) == false)
> goto out;
>
> - phy->mode = mode;
> -
> - gpio_set_value(phy->gpio_en, 1);
> - gpio_set_value(phy->gpio_fw_wake, 0);
> - if (mode == S3FWRN5_MODE_FW)
> - gpio_set_value(phy->gpio_fw_wake, 1);
> -
> - if (mode != S3FWRN5_MODE_COLD) {
> - msleep(S3FWRN5_EN_WAIT_TIME);
> - gpio_set_value(phy->gpio_en, 0);
> - msleep(S3FWRN5_EN_WAIT_TIME);
> - }
> -
> phy->irq_skip = true;
>
> out:
> - mutex_unlock(>mutex);
> -}
> -
> -static enum s3fwrn5_mode s3fwrn5_i2c_get_mode(void *phy_id)
> -{
> - struct s3fwrn5_i2c_phy *phy = phy_id;
> - enum s3fwrn5_mode mode;
> -
> - mutex_lock(>mutex);
> -
> - mode = phy->mode;
> -
> - mutex_unlock(>mutex);
> -
> - return mode;
> + mutex_unlock(>common.mutex);
> }
>
> static int s3fwrn5_i2c_write(void *phy_id, struct sk_buff *skb)
> @@ -91,7 +46,7 @@ static int s3fwrn5_i2c_write(void *phy_id, struct sk_buff
> *skb)
> struct s3fwrn5_i2c_phy *phy = phy_id;
> int ret;
>
> - mutex_lock(>mutex);
> + mutex_lock(>common.mutex);
>
> phy->irq_skip = false;
>
> @@ -102,7 +57,7 @@ static int s3fwrn5_i2c_write(void *phy_id, struct sk_buff
> *skb)
> ret = i2c_master_send(phy->i2c_dev, skb->data, skb->len);
> }
>
> - mutex_unlock(>mutex);
> + mutex_unlock(>common.mutex);
>
> if (ret < 0)
> return ret;
> @@ -114,9 +69,9 @@ static int s3fwrn5_i2c_write(void *phy_id, struct sk_buff
> *skb)
> }
>
> static const struct s3fwrn5_phy_ops i2c_phy_ops = {
> - .set_wake = s3fwrn5_i2c_set_wake,
> + .set_wake = s3fwrn5_phy_set_wake,
> .set_mode = s3fwrn5_i2c_set_mode,
> - .get_mode = s3fwrn5_i2c_get_mode,
> + .get_mode = s3fwrn5_phy_get_mode,
> .write = s3fwrn5_i2c_write,
> };
>
> @@ -128,7 +83,7 @@ static int s3fwrn5_i2c_read(struct s3fwrn5_i2c_phy *phy)
> char hdr[4];
> int ret;
>
> - hdr_size = (phy->mode == S3FWRN5_MODE_NCI) ?
> + hdr_size = (phy->common.mode == S3FWRN5_MODE_NCI) ?
> NCI_CTRL_HDR_SIZE : S3FWRN5_FW_HDR_SIZE;
> ret = i2c_master_recv(phy->i2c_dev, hdr, hdr_size);
> if (ret < 0)
> @@ -137,7 +92,7 @@ static int s3fwrn5_i2c_read(struct s3fwrn5_i2c_phy *phy)
> if (ret < hdr_size)
> return -EBADMSG;
>
> - data_len = (phy->mode == S3FWRN5_MODE_NCI) ?
> + data_len = (phy->common.mode == S3FWRN5_MODE_NCI) ?
> ((struct nci_ctrl_hdr *)hdr)->plen :
> ((struct s3fwrn5_fw_header *)hdr)->len;
>
> @@ -157,24 +112,24 @@ static int s3fwrn5_i2c_read(struct s3fwrn5_i2c_phy *phy)
> }
>
> out:
> - return s3fwrn5_recv_frame(phy->ndev, skb, phy->mode);
> + return s3fwrn5_recv_frame(phy->common.ndev, skb, phy->common.mode);
> }
>
> static irqreturn_t s3fwrn5_i2c_irq_thread_fn(int irq, void *phy_id)
> {
> struct s3fwrn5_i2c_phy *phy = phy_id;
>
> - if (!phy || !phy->ndev) {
> + if (!phy || !phy->common.ndev) {
> WARN_ON_ONCE(1);
> return IRQ_NONE;
> }
>
> - mutex_lock(>mutex);
> + mutex_lock(>common.mutex);
>
> if (phy->irq_skip)
> goto
