[PATCH v3 0/4] LVDS Controller Support for SAM9X75 SoC

[PATCH v3 0/4] LVDS Controller Support for SAM9X75 SoC

[Dharma Balasubiramani]

This patch series introduces LVDS controller support for the SAM9X75 SoC. The
LVDS controller is designed to work with Microchip's sam9x7 series
System-on-Chip (SoC) devices, providing Low Voltage Differential Signaling
capabilities.

Patch series Changelog:
- Include configs: at91: Enable LVDS serializer

The Individual Changelogs are available on the respective patches.

Dharma Balasubiramani (4):
  dt-bindings: display: bridge: add sam9x75-lvds compatible
  drm/bridge: add lvds controller support for sam9x7
  MAINTAINERS: add SAM9X7 SoC's LVDS controller
  ARM: configs: at91: Enable LVDS serializer support

 .../bridge/microchip,sam9x75-lvds.yaml        |  55 +++++
 MAINTAINERS                                   |   8 +
 arch/arm/configs/at91_dt_defconfig            |   1 +
 drivers/gpu/drm/bridge/Kconfig                |   7 +
 drivers/gpu/drm/bridge/Makefile               |   1 +
 drivers/gpu/drm/bridge/microchip-lvds.c       | 228 ++++++++++++++++++
 6 files changed, 300 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/bridge/microchip,sam9x75-lvds.yaml
 create mode 100644 drivers/gpu/drm/bridge/microchip-lvds.c


base-commit: 0dd3ee31125508cd67f7e7172247f05b7fd1753a
-- 
2.25.1

[PATCH v3 1/4] dt-bindings: display: bridge: add sam9x75-lvds compatible

[Dharma Balasubiramani]

Add the 'sam9x75-lvds' compatible binding, which describes the Low Voltage
Differential Signaling (LVDS) Controller found on some Microchip's sam9x7
series System-on-Chip (SoC) devices. This binding will be used to define
the properties and configuration for the LVDS Controller in DT.

Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
Changelog
v2 -> v3
- No changes.
v1 -> v2
- Remove '|' in description, as there is no formatting to preserve.
- Remove 'gclk' from clock-names as there is only one clock(pclk).
- Remove the unused headers and include only used ones.
- Change the compatible name specific to SoC (sam9x75) instead of entire series.
- Change file name to match the compatible name.
---
 .../bridge/microchip,sam9x75-lvds.yaml        | 55 +++++++++++++++++++
 1 file changed, 55 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/bridge/microchip,sam9x75-lvds.yaml

diff --git a/Documentation/devicetree/bindings/display/bridge/microchip,sam9x75-lvds.yaml b/Documentation/devicetree/bindings/display/bridge/microchip,sam9x75-lvds.yaml
new file mode 100644
index 000000000000..862ef441ac9f
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/microchip,sam9x75-lvds.yaml
@@ -0,0 +1,55 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/bridge/microchip,sam9x75-lvds.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip SAM9X75 LVDS Controller
+
+maintainers:
+  - Dharma Balasubiramani <dharma.b@microchip.com>
+
+description:
+  The Low Voltage Differential Signaling Controller (LVDSC) manages data
+  format conversion from the LCD Controller internal DPI bus to OpenLDI
+  LVDS output signals. LVDSC functions include bit mapping, balanced mode
+  management, and serializer.
+
+properties:
+  compatible:
+    const: microchip,sam9x75-lvds
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: Peripheral Bus Clock
+
+  clock-names:
+    items:
+      - const: pclk
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/clock/at91.h>
+    lvds-controller@f8060000 {
+      compatible = "microchip,sam9x75-lvds";
+      reg = <0xf8060000 0x100>;
+      interrupts = <56 IRQ_TYPE_LEVEL_HIGH 0>;
+      clocks = <&pmc PMC_TYPE_PERIPHERAL 56>;
+      clock-names = "pclk";
+    };
-- 
2.25.1

[PATCH v3 2/4] drm/bridge: add lvds controller support for sam9x7

[Dharma Balasubiramani]

Add a new LVDS controller driver for sam9x7 which does the following:
- Prepares and enables the LVDS Peripheral clock
- Defines its connector type as DRM_MODE_CONNECTOR_LVDS and adds itself
to the global bridge list.
- Identifies its output endpoint as panel and adds it to the encoder
display pipeline
- Enables the LVDS serializer

Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
---
Changelog
v2 ->v3
- Correct Typo error "serializer".
- Consolidate get() and prepare() functions and use devm_clk_get_prepared().
- Remove unused variable 'ret' in probe().
- Use devm_pm_runtime_enable() and drop the mchp_lvds_remove().

v1 -> v2
- Drop 'res' variable and combine two lines into one.
- Handle deferred probe properly, use dev_err_probe().
- Don't print anything on deferred probe. Dropped print.
- Remove the MODULE_ALIAS and add MODULE_DEVICE_TABLE().
- symbol 'mchp_lvds_driver' was not declared. It should be static.
---
 drivers/gpu/drm/bridge/Kconfig          |   7 +
 drivers/gpu/drm/bridge/Makefile         |   1 +
 drivers/gpu/drm/bridge/microchip-lvds.c | 228 ++++++++++++++++++++++++
 3 files changed, 236 insertions(+)
 create mode 100644 drivers/gpu/drm/bridge/microchip-lvds.c

diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index 3e6a4e2044c0..74ca0edb4e0d 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -173,6 +173,13 @@ config DRM_MEGACHIPS_STDPXXXX_GE_B850V3_FW
 	  to DP++. This is used with the i.MX6 imx-ldb
 	  driver. You are likely to say N here.
 
+config DRM_MICROCHIP_LVDS_SERIALIZER
+	tristate "Microchip LVDS serializer support"
+	depends on OF
+	depends on DRM_ATMEL_HLCDC
+	help
+	  Support for Microchip's LVDS serializer.
+
 config DRM_NWL_MIPI_DSI
 	tristate "Northwest Logic MIPI DSI Host controller"
 	depends on DRM
diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
index 2b892b7ed59e..e3804e93d324 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -11,6 +11,7 @@ obj-$(CONFIG_DRM_LONTIUM_LT9611) += lontium-lt9611.o
 obj-$(CONFIG_DRM_LONTIUM_LT9611UXC) += lontium-lt9611uxc.o
 obj-$(CONFIG_DRM_LVDS_CODEC) += lvds-codec.o
 obj-$(CONFIG_DRM_MEGACHIPS_STDPXXXX_GE_B850V3_FW) += megachips-stdpxxxx-ge-b850v3-fw.o
+obj-$(CONFIG_DRM_MICROCHIP_LVDS_SERIALIZER) += microchip-lvds.o
 obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o
 obj-$(CONFIG_DRM_PARADE_PS8622) += parade-ps8622.o
 obj-$(CONFIG_DRM_PARADE_PS8640) += parade-ps8640.o
diff --git a/drivers/gpu/drm/bridge/microchip-lvds.c b/drivers/gpu/drm/bridge/microchip-lvds.c
new file mode 100644
index 000000000000..d3fd9d722e36
--- /dev/null
+++ b/drivers/gpu/drm/bridge/microchip-lvds.c
@@ -0,0 +1,228 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2023 Microchip Technology Inc. and its subsidiaries
+ *
+ * Author: Manikandan Muralidharan <manikandan.m@microchip.com>
+ * Author: Dharma Balasubiramani <dharma.b@microchip.com>
+ *
+ */
+
+#include <linux/clk.h>
+#include <linux/component.h>
+#include <linux/delay.h>
+#include <linux/jiffies.h>
+#include <linux/mfd/syscon.h>
+#include <linux/of_graph.h>
+#include <linux/pinctrl/devinfo.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
+#include <linux/reset.h>
+
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_bridge.h>
+#include <drm/drm_of.h>
+#include <drm/drm_panel.h>
+#include <drm/drm_print.h>
+#include <drm/drm_probe_helper.h>
+#include <drm/drm_simple_kms_helper.h>
+
+#define LVDS_POLL_TIMEOUT_MS 1000
+
+/* LVDSC register offsets */
+#define LVDSC_CR	0x00
+#define LVDSC_CFGR	0x04
+#define LVDSC_SR	0x0C
+#define LVDSC_WPMR	0xE4
+
+/* Bitfields in LVDSC_CR (Control Register) */
+#define LVDSC_CR_SER_EN	BIT(0)
+
+/* Bitfields in LVDSC_CFGR (Configuration Register) */
+#define LVDSC_CFGR_PIXSIZE_24BITS	0
+#define LVDSC_CFGR_DEN_POL_HIGH		0
+#define LVDSC_CFGR_DC_UNBALANCED	0
+#define LVDSC_CFGR_MAPPING_JEIDA	BIT(6)
+
+/*Bitfields in LVDSC_SR */
+#define LVDSC_SR_CS	BIT(0)
+
+/* Bitfields in LVDSC_WPMR (Write Protection Mode Register) */
+#define LVDSC_WPMR_WPKEY_MASK	GENMASK(31, 8)
+#define LVDSC_WPMR_WPKEY_PSSWD	0x4C5644
+
+struct mchp_lvds {
+	struct device *dev;
+	void __iomem *regs;
+	struct clk *pclk;
+	int format; /* vesa or jeida format */
+	struct drm_panel *panel;
+	struct drm_bridge bridge;
+	struct drm_bridge *panel_bridge;
+};
+
+static inline struct mchp_lvds *bridge_to_lvds(struct drm_bridge *bridge)
+{
+	return container_of(bridge, struct mchp_lvds, bridge);
+}
+
+static inline u32 lvds_readl(struct mchp_lvds *lvds, u32 offset)
+{
+	return readl_relaxed(lvds->regs + offset);
+}
+
+static inline void lvds_writel(struct mchp_lvds *lvds, u32 offset, u32 val)
+{
+	writel_relaxed(val, lvds->regs + offset);
+}
+
+static void lvds_serialiser_on(struct mchp_lvds *lvds)
+{
+	unsigned long timeout = jiffies + msecs_to_jiffies(LVDS_POLL_TIMEOUT_MS);
+
+	/* The LVDSC registers can only be written if WPEN is cleared */
+	lvds_writel(lvds, LVDSC_WPMR, (LVDSC_WPMR_WPKEY_PSSWD &
+				LVDSC_WPMR_WPKEY_MASK));
+
+	/* Wait for the status of configuration registers to be changed */
+	while (lvds_readl(lvds, LVDSC_SR) & LVDSC_SR_CS) {
+		if (time_after(jiffies, timeout)) {
+			dev_err(lvds->dev, "%s: timeout error\n", __func__);
+			return;
+		}
+		usleep_range(1000, 2000);
+	}
+
+	/* Configure the LVDSC */
+	lvds_writel(lvds, LVDSC_CFGR, (LVDSC_CFGR_MAPPING_JEIDA |
+				LVDSC_CFGR_DC_UNBALANCED |
+				LVDSC_CFGR_DEN_POL_HIGH |
+				LVDSC_CFGR_PIXSIZE_24BITS));
+
+	/* Enable the LVDS serializer */
+	lvds_writel(lvds, LVDSC_CR, LVDSC_CR_SER_EN);
+}
+
+static int mchp_lvds_attach(struct drm_bridge *bridge,
+			    enum drm_bridge_attach_flags flags)
+{
+	struct mchp_lvds *lvds = bridge_to_lvds(bridge);
+
+	bridge->encoder->encoder_type = DRM_MODE_ENCODER_LVDS;
+
+	return drm_bridge_attach(bridge->encoder, lvds->panel_bridge,
+				 bridge, flags);
+}
+
+static void mchp_lvds_enable(struct drm_bridge *bridge)
+{
+	struct mchp_lvds *lvds = bridge_to_lvds(bridge);
+	int ret;
+
+	ret = clk_enable(lvds->pclk);
+	if (ret < 0) {
+		DRM_DEV_ERROR(lvds->dev, "failed to enable lvds pclk %d\n", ret);
+		return;
+	}
+
+	ret = pm_runtime_get_sync(lvds->dev);
+	if (ret < 0) {
+		DRM_DEV_ERROR(lvds->dev, "failed to get pm runtime: %d\n", ret);
+		clk_disable(lvds->pclk);
+		return;
+	}
+
+	lvds_serialiser_on(lvds);
+}
+
+static void mchp_lvds_disable(struct drm_bridge *bridge)
+{
+	struct mchp_lvds *lvds = bridge_to_lvds(bridge);
+
+	pm_runtime_put(lvds->dev);
+	clk_disable(lvds->pclk);
+}
+
+static const struct drm_bridge_funcs mchp_lvds_bridge_funcs = {
+	.attach = mchp_lvds_attach,
+	.enable = mchp_lvds_enable,
+	.disable = mchp_lvds_disable,
+};
+
+static int mchp_lvds_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct mchp_lvds *lvds;
+	struct device_node *port;
+
+	if (!dev->of_node)
+		return -ENODEV;
+
+	lvds = devm_kzalloc(&pdev->dev, sizeof(*lvds), GFP_KERNEL);
+	if (!lvds)
+		return -ENOMEM;
+
+	lvds->dev = dev;
+
+	lvds->regs = devm_ioremap_resource(lvds->dev,
+			platform_get_resource(pdev, IORESOURCE_MEM, 0));
+	if (IS_ERR(lvds->regs))
+		return PTR_ERR(lvds->regs);
+
+	lvds->pclk = devm_clk_get_prepared(lvds->dev, "pclk");
+	if (IS_ERR(lvds->pclk))
+		return dev_err_probe(lvds->dev, PTR_ERR(lvds->pclk),
+				"could not get pclk_lvds prepared\n");
+
+	port = of_graph_get_remote_node(dev->of_node, 1, 0);
+	if (!port) {
+		DRM_DEV_ERROR(dev,
+			      "can't find port point, please init lvds panel port!\n");
+		return -EINVAL;
+	}
+
+	lvds->panel = of_drm_find_panel(port);
+	of_node_put(port);
+
+	if (IS_ERR(lvds->panel))
+		return -EPROBE_DEFER;
+
+	lvds->panel_bridge = devm_drm_panel_bridge_add(dev, lvds->panel);
+
+	if (IS_ERR(lvds->panel_bridge))
+		return PTR_ERR(lvds->panel_bridge);
+
+	lvds->bridge.of_node = dev->of_node;
+	lvds->bridge.type = DRM_MODE_CONNECTOR_LVDS;
+	lvds->bridge.funcs = &mchp_lvds_bridge_funcs;
+
+	dev_set_drvdata(dev, lvds);
+	devm_pm_runtime_enable(dev);
+
+	drm_bridge_add(&lvds->bridge);
+
+	return 0;
+}
+
+static const struct of_device_id mchp_lvds_dt_ids[] = {
+	{
+		.compatible = "microchip,sam9x75-lvds",
+	},
+	{},
+};
+MODULE_DEVICE_TABLE(of, mchp_lvds_dt_ids);
+
+static struct platform_driver mchp_lvds_driver = {
+	.probe = mchp_lvds_probe,
+	.driver = {
+		   .name = "microchip-lvds",
+		   .of_match_table = mchp_lvds_dt_ids,
+	},
+};
+module_platform_driver(mchp_lvds_driver);
+
+MODULE_AUTHOR("Manikandan Muralidharan <manikandan.m@microchip.com>");
+MODULE_AUTHOR("Dharma Balasubiramani <dharma.b@microchip.com>");
+MODULE_DESCRIPTION("Low Voltage Differential Signaling Controller Driver");
+MODULE_LICENSE("GPL");
-- 
2.25.1

[PATCH v3 3/4] MAINTAINERS: add SAM9X7 SoC's LVDS controller

[Dharma Balasubiramani]

Add the newly added LVDS controller for the SAM9X7 SoC to the existing
MAINTAINERS entry.

Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>
---
Changelog
v2 -> v3
- Move the entry before "MICROCHIP SAMA5D2-COMPATIBLE ADC DRIVER".

v1 -> v2
- No Changes.
---
 MAINTAINERS | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index a7c4cf8201e0..ce592b6cf375 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14216,6 +14216,14 @@ S:	Supported
 F:	Documentation/devicetree/bindings/pwm/atmel,at91sam-pwm.yaml
 F:	drivers/pwm/pwm-atmel.c
 
+MICROCHIP SAM9x7-COMPATIBLE LVDS CONTROLLER
+M:	Manikandan Muralidharan <manikandan.m@microchip.com>
+M:	Dharma Balasubiramani <dharma.b@microchip.com>
+L:	dri-devel@lists.freedesktop.org
+S:	Supported
+F:	Documentation/devicetree/bindings/display/bridge/microchip,sam9x7-lvds.yaml
+F:	drivers/gpu/drm/bridge/microchip-lvds.c
+
 MICROCHIP SAMA5D2-COMPATIBLE ADC DRIVER
 M:	Eugen Hristev <eugen.hristev@microchip.com>
 L:	linux-iio@vger.kernel.org
-- 
2.25.1

[PATCH v3 4/4] ARM: configs: at91: Enable LVDS serializer support

[Dharma Balasubiramani]

Enable LVDS serializer support for display pipeline.

Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
Acked-by: Hari Prasath Gujulan Elango <hari.prasathge@microchip.com>
Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>
---
Changelog
v2 -> v3
- No Changes.
---
 arch/arm/configs/at91_dt_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/configs/at91_dt_defconfig b/arch/arm/configs/at91_dt_defconfig
index 71b5acc78187..6a7714beb099 100644
--- a/arch/arm/configs/at91_dt_defconfig
+++ b/arch/arm/configs/at91_dt_defconfig
@@ -143,6 +143,7 @@ CONFIG_VIDEO_OV2640=m
 CONFIG_VIDEO_OV7740=m
 CONFIG_DRM=y
 CONFIG_DRM_ATMEL_HLCDC=y
+CONFIG_DRM_MICROCHIP_LVDS_SERIALIZER=y
 CONFIG_DRM_PANEL_SIMPLE=y
 CONFIG_DRM_PANEL_EDP=y
 CONFIG_FB_ATMEL=y
-- 
2.25.1

Re: [PATCH v3 1/4] dt-bindings: display: bridge: add sam9x75-lvds compatible

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 217 bytes --]

$subject: dt-bindings: display: bridge: add sam9x75-lvds compatible

If there's a respin for some reason, please update the subject to match
what the commit is actually doing (adding a whole binding).

Cheers,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v3 1/4] dt-bindings: display: bridge: add sam9x75-lvds compatible

[Krzysztof Kozlowski]

On 07/02/2024 11:27, Dharma Balasubiramani wrote:
> Add the 'sam9x75-lvds' compatible binding, which describes the Low Voltage
> Differential Signaling (LVDS) Controller found on some Microchip's sam9x7
> series System-on-Chip (SoC) devices. This binding will be used to define
> the properties and configuration for the LVDS Controller in DT.
> 
> Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>

Not tested...

Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC. It might happen, that command when run on an older
kernel, gives you outdated entries. Therefore please be sure you base
your patches on recent Linux kernel.

Tools like b4 or scripts/get_maintainer.pl provide you proper list of
people, so fix your workflow. Tools might also fail if you work on some
ancient tree (don't, instead use mainline), work on fork of kernel
(don't, instead use mainline) or you ignore some maintainers (really
don't). Just use b4 and everything should be fine, although remember
about `b4 prep --auto-to-cc` if you added new patches to the patchset.

You missed at least devicetree list (maybe more), so this won't be
tested by automated tooling. Performing review on untested code might be
a waste of time.

Please kindly resend and include all necessary To/Cc entries.


Best regards,
Krzysztof

Re: [PATCH v3 1/4] dt-bindings: display: bridge: add sam9x75-lvds compatible

[Dharma.B]

On 08/02/24 2:31 pm, Krzysztof Kozlowski wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On 07/02/2024 11:27, Dharma Balasubiramani wrote:
>> Add the 'sam9x75-lvds' compatible binding, which describes the Low Voltage
>> Differential Signaling (LVDS) Controller found on some Microchip's sam9x7
>> series System-on-Chip (SoC) devices. This binding will be used to define
>> the properties and configuration for the LVDS Controller in DT.
>>
>> Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
> 
> Not tested...
> 
> Please use scripts/get_maintainers.pl to get a list of necessary people
> and lists to CC. It might happen, that command when run on an older
> kernel, gives you outdated entries. Therefore please be sure you base
> your patches on recent Linux kernel.
> 
> Tools like b4 or scripts/get_maintainer.pl provide you proper list of
> people, so fix your workflow. Tools might also fail if you work on some
> ancient tree (don't, instead use mainline), work on fork of kernel
> (don't, instead use mainline) or you ignore some maintainers (really
> don't). Just use b4 and everything should be fine, although remember
> about `b4 prep --auto-to-cc` if you added new patches to the patchset.
> 
> You missed at least devicetree list (maybe more), so this won't be
> tested by automated tooling. Performing review on untested code might be
> a waste of time.

Apologies for the oversight, somehow it got missed.
> 
> Please kindly resend and include all necessary To/Cc entries.

I will resend the series including all necessary To/Cc entries.

Thanks.
> 
> 
> Best regards,
> Krzysztof
> 

-- 
With Best Regards,
Dharma B.

Re: [PATCH v3 1/4] dt-bindings: display: bridge: add sam9x75-lvds compatible

[Dharma.B]

Hi Conor,

On 07/02/24 9:22 pm, Conor Dooley wrote:
> $subject: dt-bindings: display: bridge: add sam9x75-lvds compatible
> 
> If there's a respin for some reason, please update the subject to match
> what the commit is actually doing (adding a whole binding).

Absolutely, I'll make the adjustment as follows: "dt-bindings: display: 
bridge: add sam9x75-lvds binding"
> 
> Cheers,
> Conor.

-- 
With Best Regards,
Dharma B.

Re: [PATCH v3 1/4] dt-bindings: display: bridge: add sam9x75-lvds compatible

[Dharma.B]

On 09/02/24 7:50 pm, Dharma B wrote:
> On 08/02/24 2:31 pm, Krzysztof Kozlowski wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know 
>> the content is safe
>>
>> On 07/02/2024 11:27, Dharma Balasubiramani wrote:
>>> Add the 'sam9x75-lvds' compatible binding, which describes the Low 
>>> Voltage
>>> Differential Signaling (LVDS) Controller found on some Microchip's 
>>> sam9x7
>>> series System-on-Chip (SoC) devices. This binding will be used to define
>>> the properties and configuration for the LVDS Controller in DT.
>>>
>>> Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
>>
>> Not tested...
>>
>> Please use scripts/get_maintainers.pl to get a list of necessary people
>> and lists to CC. It might happen, that command when run on an older
>> kernel, gives you outdated entries. Therefore please be sure you base
>> your patches on recent Linux kernel.
>>
>> Tools like b4 or scripts/get_maintainer.pl provide you proper list of
>> people, so fix your workflow. Tools might also fail if you work on some
>> ancient tree (don't, instead use mainline), work on fork of kernel
>> (don't, instead use mainline) or you ignore some maintainers (really
>> don't). Just use b4 and everything should be fine, although remember
>> about `b4 prep --auto-to-cc` if you added new patches to the patchset.
>>
>> You missed at least devicetree list (maybe more), so this won't be
>> tested by automated tooling. Performing review on untested code might be
>> a waste of time.
> 
> Apologies for the oversight, somehow it got missed.

The get_maintainer.pl seems to be inconsistent with the results.

linux$ ./scripts/get_maintainer.pl *patch | wc -l
./scripts/get_maintainer.pl: file '0000-cover-letter.patch' doesn't 
appear to be a patch.  Add -f to options?
31
linux$ ./scripts/get_maintainer.pl *patch | wc -l
./scripts/get_maintainer.pl: file '0000-cover-letter.patch' doesn't 
appear to be a patch.  Add -f to options?
29
linux$ ./scripts/get_maintainer.pl *patch | wc -l
./scripts/get_maintainer.pl: file '0000-cover-letter.patch' doesn't 
appear to be a patch.  Add -f to options?
30
linux$ ./scripts/get_maintainer.pl *patch | wc -l
./scripts/get_maintainer.pl: file '0000-cover-letter.patch' doesn't 
appear to be a patch.  Add -f to options?
30

Anyway, I will add the largest recipients list in v4.
>>
>> Please kindly resend and include all necessary To/Cc entries.
> 
> I will resend the series including all necessary To/Cc entries.
> 
> Thanks.
>>
>>
>> Best regards,
>> Krzysztof
>>
> 

-- 
With Best Regards,
Dharma B.

Re: [PATCH v3 1/4] dt-bindings: display: bridge: add sam9x75-lvds compatible

[Krzysztof Kozlowski]

On 09/02/2024 16:02, Dharma.B@microchip.com wrote:
> On 09/02/24 7:50 pm, Dharma B wrote:
>> On 08/02/24 2:31 pm, Krzysztof Kozlowski wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know 
>>> the content is safe
>>>
>>> On 07/02/2024 11:27, Dharma Balasubiramani wrote:
>>>> Add the 'sam9x75-lvds' compatible binding, which describes the Low 
>>>> Voltage
>>>> Differential Signaling (LVDS) Controller found on some Microchip's 
>>>> sam9x7
>>>> series System-on-Chip (SoC) devices. This binding will be used to define
>>>> the properties and configuration for the LVDS Controller in DT.
>>>>
>>>> Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
>>>
>>> Not tested...
>>>
>>> Please use scripts/get_maintainers.pl to get a list of necessary people
>>> and lists to CC. It might happen, that command when run on an older
>>> kernel, gives you outdated entries. Therefore please be sure you base
>>> your patches on recent Linux kernel.
>>>
>>> Tools like b4 or scripts/get_maintainer.pl provide you proper list of
>>> people, so fix your workflow. Tools might also fail if you work on some
>>> ancient tree (don't, instead use mainline), work on fork of kernel
>>> (don't, instead use mainline) or you ignore some maintainers (really
>>> don't). Just use b4 and everything should be fine, although remember
>>> about `b4 prep --auto-to-cc` if you added new patches to the patchset.
>>>
>>> You missed at least devicetree list (maybe more), so this won't be
>>> tested by automated tooling. Performing review on untested code might be
>>> a waste of time.
>>
>> Apologies for the oversight, somehow it got missed.
> 
> The get_maintainer.pl seems to be inconsistent with the results.
> 
> linux$ ./scripts/get_maintainer.pl *patch | wc -l
> ./scripts/get_maintainer.pl: file '0000-cover-letter.patch' doesn't 
> appear to be a patch.  Add -f to options?
> 31
> linux$ ./scripts/get_maintainer.pl *patch | wc -l
> ./scripts/get_maintainer.pl: file '0000-cover-letter.patch' doesn't 
> appear to be a patch.  Add -f to options?
> 29
> linux$ ./scripts/get_maintainer.pl *patch | wc -l
> ./scripts/get_maintainer.pl: file '0000-cover-letter.patch' doesn't 
> appear to be a patch.  Add -f to options?
> 30
> linux$ ./scripts/get_maintainer.pl *patch | wc -l
> ./scripts/get_maintainer.pl: file '0000-cover-letter.patch' doesn't 
> appear to be a patch.  Add -f to options?
> 30

Why would you add 30 addresses, including many unrelated people, to the
cc-list? You must add only maintainers (so also reviewers) and mailing
lists.

Best regards,
Krzysztof

Re: [PATCH v3 1/4] dt-bindings: display: bridge: add sam9x75-lvds compatible

[Krzysztof Kozlowski]

On 09/02/2024 17:05, Krzysztof Kozlowski wrote:
> On 09/02/2024 16:02, Dharma.B@microchip.com wrote:
>> On 09/02/24 7:50 pm, Dharma B wrote:
>>> On 08/02/24 2:31 pm, Krzysztof Kozlowski wrote:
>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know 
>>>> the content is safe
>>>>
>>>> On 07/02/2024 11:27, Dharma Balasubiramani wrote:
>>>>> Add the 'sam9x75-lvds' compatible binding, which describes the Low 
>>>>> Voltage
>>>>> Differential Signaling (LVDS) Controller found on some Microchip's 
>>>>> sam9x7
>>>>> series System-on-Chip (SoC) devices. This binding will be used to define
>>>>> the properties and configuration for the LVDS Controller in DT.
>>>>>
>>>>> Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
>>>>
>>>> Not tested...
>>>>
>>>> Please use scripts/get_maintainers.pl to get a list of necessary people
>>>> and lists to CC. It might happen, that command when run on an older
>>>> kernel, gives you outdated entries. Therefore please be sure you base
>>>> your patches on recent Linux kernel.
>>>>
>>>> Tools like b4 or scripts/get_maintainer.pl provide you proper list of
>>>> people, so fix your workflow. Tools might also fail if you work on some
>>>> ancient tree (don't, instead use mainline), work on fork of kernel
>>>> (don't, instead use mainline) or you ignore some maintainers (really
>>>> don't). Just use b4 and everything should be fine, although remember
>>>> about `b4 prep --auto-to-cc` if you added new patches to the patchset.
>>>>
>>>> You missed at least devicetree list (maybe more), so this won't be
>>>> tested by automated tooling. Performing review on untested code might be
>>>> a waste of time.
>>>
>>> Apologies for the oversight, somehow it got missed.
>>
>> The get_maintainer.pl seems to be inconsistent with the results.
>>
>> linux$ ./scripts/get_maintainer.pl *patch | wc -l
>> ./scripts/get_maintainer.pl: file '0000-cover-letter.patch' doesn't 
>> appear to be a patch.  Add -f to options?
>> 31
>> linux$ ./scripts/get_maintainer.pl *patch | wc -l
>> ./scripts/get_maintainer.pl: file '0000-cover-letter.patch' doesn't 
>> appear to be a patch.  Add -f to options?
>> 29
>> linux$ ./scripts/get_maintainer.pl *patch | wc -l
>> ./scripts/get_maintainer.pl: file '0000-cover-letter.patch' doesn't 
>> appear to be a patch.  Add -f to options?
>> 30
>> linux$ ./scripts/get_maintainer.pl *patch | wc -l
>> ./scripts/get_maintainer.pl: file '0000-cover-letter.patch' doesn't 
>> appear to be a patch.  Add -f to options?
>> 30
> 
> Why would you add 30 addresses, including many unrelated people, to the
> cc-list? You must add only maintainers (so also reviewers) and mailing
> lists.

Really, why do you Cc MM folks on this patch? Just read what
get_maintainer.pl tells you, e.g. when it says that someone made one
commit to maintainers file, shall this person be Cc-ed? No, it should be
obvious...

Best regards,
Krzysztof

Re: [PATCH v3 1/4] dt-bindings: display: bridge: add sam9x75-lvds compatible

[Dharma.B]

Hi Krzysztof,

On 09/02/24 9:40 pm, Krzysztof Kozlowski wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On 09/02/2024 17:05, Krzysztof Kozlowski wrote:
>> On 09/02/2024 16:02, Dharma.B@microchip.com wrote:
>>> On 09/02/24 7:50 pm, Dharma B wrote:
>>>> On 08/02/24 2:31 pm, Krzysztof Kozlowski wrote:
>>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know
>>>>> the content is safe
>>>>>
>>>>> On 07/02/2024 11:27, Dharma Balasubiramani wrote:
>>>>>> Add the 'sam9x75-lvds' compatible binding, which describes the Low
>>>>>> Voltage
>>>>>> Differential Signaling (LVDS) Controller found on some Microchip's
>>>>>> sam9x7
>>>>>> series System-on-Chip (SoC) devices. This binding will be used to define
>>>>>> the properties and configuration for the LVDS Controller in DT.
>>>>>>
>>>>>> Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
>>>>>
>>>>> Not tested...
>>>>>
>>>>> Please use scripts/get_maintainers.pl to get a list of necessary people
>>>>> and lists to CC. It might happen, that command when run on an older
>>>>> kernel, gives you outdated entries. Therefore please be sure you base
>>>>> your patches on recent Linux kernel.
>>>>>
>>>>> Tools like b4 or scripts/get_maintainer.pl provide you proper list of
>>>>> people, so fix your workflow. Tools might also fail if you work on some
>>>>> ancient tree (don't, instead use mainline), work on fork of kernel
>>>>> (don't, instead use mainline) or you ignore some maintainers (really
>>>>> don't). Just use b4 and everything should be fine, although remember
>>>>> about `b4 prep --auto-to-cc` if you added new patches to the patchset.
>>>>>
>>>>> You missed at least devicetree list (maybe more), so this won't be
>>>>> tested by automated tooling. Performing review on untested code might be
>>>>> a waste of time.
>>>>
>>>> Apologies for the oversight, somehow it got missed.
>>>
>>> The get_maintainer.pl seems to be inconsistent with the results.
>>>
>>> linux$ ./scripts/get_maintainer.pl *patch | wc -l
>>> ./scripts/get_maintainer.pl: file '0000-cover-letter.patch' doesn't
>>> appear to be a patch.  Add -f to options?
>>> 31
>>> linux$ ./scripts/get_maintainer.pl *patch | wc -l
>>> ./scripts/get_maintainer.pl: file '0000-cover-letter.patch' doesn't
>>> appear to be a patch.  Add -f to options?
>>> 29
>>> linux$ ./scripts/get_maintainer.pl *patch | wc -l
>>> ./scripts/get_maintainer.pl: file '0000-cover-letter.patch' doesn't
>>> appear to be a patch.  Add -f to options?
>>> 30
>>> linux$ ./scripts/get_maintainer.pl *patch | wc -l
>>> ./scripts/get_maintainer.pl: file '0000-cover-letter.patch' doesn't
>>> appear to be a patch.  Add -f to options?
>>> 30
>>
>> Why would you add 30 addresses, including many unrelated people, to the
>> cc-list? You must add only maintainers (so also reviewers) and mailing
>> lists.
> 
> Really, why do you Cc MM folks on this patch? Just read what
> get_maintainer.pl tells you, e.g. when it says that someone made one
> commit to maintainers file, shall this person be Cc-ed? No, it should be
> obvious...


Thank you for bringing this to my attention. I sincerely apologize for 
any oversight. Moving forward, I will ensure to utilize 'b4 prep 
--auto-to-cc' to obtain the accurate To and CC list.

> 
> Best regards,
> Krzysztof
> 

-- 
With Best Regards,
Dharma B.

Re: [PATCH v3 4/4] ARM: configs: at91: Enable LVDS serializer support

[Krzysztof Kozlowski]

On 07/02/2024 11:28, Dharma Balasubiramani wrote:
> Enable LVDS serializer support for display pipeline.
> 
> Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
> Acked-by: Hari Prasath Gujulan Elango <hari.prasathge@microchip.com>
> Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>
> ---
> Changelog
> v2 -> v3
> - No Changes.
> ---

In other email thread you mentioned that this depends on driver changes,
while that's only partially correct. This patch does not make sense if
others are not accepted, but the others are not dependency for this one.
This should go via arm soc.

In the future, please state clearly dependencies in cover letter or each
patch changelog (so ---, not commit msg).

Best regards,
Krzysztof

Re: [PATCH v3 4/4] ARM: configs: at91: Enable LVDS serializer support

[Dharma.B]

Hi Krzysztof,

On 20/02/24 2:15 pm, Krzysztof Kozlowski wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On 07/02/2024 11:28, Dharma Balasubiramani wrote:
>> Enable LVDS serializer support for display pipeline.
>>
>> Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
>> Acked-by: Hari Prasath Gujulan Elango <hari.prasathge@microchip.com>
>> Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>
>> ---
>> Changelog
>> v2 -> v3
>> - No Changes.
>> ---
> 
> In other email thread you mentioned that this depends on driver changes,
> while that's only partially correct. This patch does not make sense if
> others are not accepted, but the others are not dependency for this one.
> This should go via arm soc.
> 
> In the future, please state clearly dependencies in cover letter or each
> patch changelog (so ---, not commit msg).

Understood, Thanks for the comprehensive explanation. I will take care 
of this in future.

> 
> Best regards,
> Krzysztof
> 

-- 
With Best Regards,
Dharma B.