[PATCH] ata: sata_mv: enable SoC SATA LED presence indication

[PATCH] ata: sata_mv: enable SoC SATA LED presence indication

[Carlos Junior]

Enable the SoC SATA LED controller in mv_soc_enable_leds().

Set the LED controller enable, device presence multiplexing, and
activity blink bits in the SoC SATA host-controller register. This
enables SATA device presence indication on boards using the SoC SATA
controller.

Signed-off-by: Carlos Junior <cjuniorfox@gmail.com>
---
 drivers/ata/sata_mv.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
index d260f04e6..4a6a6dc7b 100644
--- a/drivers/ata/sata_mv.c
+++ b/drivers/ata/sata_mv.c
@@ -251,6 +251,7 @@ enum {
 	SOC_LED_CTRL		= 0x2c,
 	SOC_LED_CTRL_BLINK	= (1 << 0),	/* Active LED blink */
 	SOC_LED_CTRL_ACT_PRESENCE = (1 << 2),	/* Multiplex dev presence */
+	SOC_LED_CTRL_ENABLE	= (1 << 3),	/* Enable SATA LED control */
 						/*  with dev activity LED */
 
 	/* Shadow block registers */
@@ -3382,12 +3383,21 @@ static void mv6_phy_errata(struct mv_host_priv *hpriv, void __iomem *mmio,
 	writel(m2, port_mmio + PHY_MODE2);
 }
 
-/* TODO: use the generic LED interface to configure the SATA Presence */
-/* & Acitivy LEDs on the board */
+/*
+ * Enable the SoC SATA LED controller and device presence
+ * indication on the board's SATA LEDs.
+ */
 static void mv_soc_enable_leds(struct mv_host_priv *hpriv,
 				      void __iomem *mmio)
 {
-	return;
+	void __iomem *hc_mmio;
+
+	hc_mmio = mv_hc_base(mmio, 0);
+
+	writel(SOC_LED_CTRL_ENABLE |
+		SOC_LED_CTRL_ACT_PRESENCE |
+		SOC_LED_CTRL_BLINK,
+		hc_mmio + SOC_LED_CTRL);
 }
 
 static void mv_soc_read_preamp(struct mv_host_priv *hpriv, int idx,
-- 
2.54.0

Re: [PATCH] ata: sata_mv: enable SoC SATA LED presence indication

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] ata: sata_mv: LED blink mode state desynchronization and register clobbering
- [Medium] ata: sata_mv: hardcoded LED multiplexing risks board-specific pin conflicts
--

commit 30e313ac1545b898b81aab6027f1bf3c7b86b831
Author: Carlos Junior <cjuniorfox@gmail.com>

ata: sata_mv: enable SoC SATA LED presence indication

This commit enables the SoC SATA LED controller in mv_soc_enable_leds().
It sets the enable, device presence multiplexing, and activity blink bits
in the host-controller register.

> diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
> index 365454390d7e0..90a4ca5b5c407 100644
> --- a/drivers/ata/sata_mv.c
> +++ b/drivers/ata/sata_mv.c
[ ... ]
> @@ -3382,12 +3383,21 @@ static void mv6_phy_errata(struct mv_host_priv *hpriv, void __iomem *mmio,
>  	writel(m2, port_mmio + PHY_MODE2);
>  }
>  
> -/* TODO: use the generic LED interface to configure the SATA Presence */
> -/* & Acitivy LEDs on the board */
> +/*
> + * Enable the SoC SATA LED controller and device presence
> + * indication on the board's SATA LEDs.
> + */
>  static void mv_soc_enable_leds(struct mv_host_priv *hpriv,
>  				      void __iomem *mmio)
>  {
> -	return;
> +	void __iomem *hc_mmio;
> +
> +	hc_mmio = mv_hc_base(mmio, 0);
> +
> +	writel(SOC_LED_CTRL_ENABLE |
> +		SOC_LED_CTRL_ACT_PRESENCE |
> +		SOC_LED_CTRL_BLINK,
> +		hc_mmio + SOC_LED_CTRL);

[Severity: High]
Does this blind writel clobber existing register configurations? Without a
read-modify-write pattern, any settings previously configured by the
bootloader will be overwritten.

Additionally, this enables blink mode in hardware without updating the
software state tracker. When mv_edma_cfg() later attempts to disable
blinking for non-NCQ devices, it relies on mv_soc_led_blink_disable():

    if (!(hpriv->hp_flags & MV_HP_QUIRK_LED_BLINK_EN))
        return;

Because hpriv->hp_flags was never updated to include
MV_HP_QUIRK_LED_BLINK_EN during initialization, this check will falsely
indicate that blinking is already disabled and return early. Will this
leave non-NCQ devices permanently stuck in blink mode?

[Severity: Medium]
Does forcing SOC_LED_CTRL_ACT_PRESENCE globally for all SoC devices risk
board-specific pin conflicts?

Unconditionally enabling LED multiplexing here overrides platform-specific
pinmux routing. On boards where these physical pins are utilized for other
GPIO functions like UART or I2C, forcing the controller to drive them
could result in hardware conflicts or erratic behavior. The previous
comment indicated this should be handled through the generic LED interface
to respect dynamic Device Tree routing.

>  }
>  
>  static void mv_soc_read_preamp(struct mv_host_priv *hpriv, int idx,

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260925193134.114566-1-cjuniorfox@gmail.com?part=1

[PATCH v2] ata: sata_mv: enable SoC SATA LED presence indication

[Carlos Junior]

Enable the SoC SATA LED controller in mv_soc_enable_leds().

Set the LED controller enable and device presence multiplexing bits
in the SoC SATA host-controller register. This enables SATA device
presence indication on boards using the SoC SATA controller.

Signed-off-by: Carlos Junior <cjuniorfox@gmail.com>
---
Changes in v2:
- Do not initialize the SoC SATA LED blink bit; leave it to the
  existing NCQ LED blink handling.

 drivers/ata/sata_mv.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
index d260f04e6..52e33485b 100644
--- a/drivers/ata/sata_mv.c
+++ b/drivers/ata/sata_mv.c
@@ -252,6 +252,7 @@ enum {
 	SOC_LED_CTRL_BLINK	= (1 << 0),	/* Active LED blink */
 	SOC_LED_CTRL_ACT_PRESENCE = (1 << 2),	/* Multiplex dev presence */
 						/*  with dev activity LED */
+	SOC_LED_CTRL_ENABLE	= (1 << 3),	/* Enable SATA LED control */
 
 	/* Shadow block registers */
 	SHD_BLK			= 0x100,
@@ -3382,12 +3383,20 @@ static void mv6_phy_errata(struct mv_host_priv *hpriv, void __iomem *mmio,
 	writel(m2, port_mmio + PHY_MODE2);
 }
 
-/* TODO: use the generic LED interface to configure the SATA Presence */
-/* & Acitivy LEDs on the board */
+/*
+ * Enable the SoC SATA LED controller and device presence
+ * indication on the board's SATA LEDs.
+ */
 static void mv_soc_enable_leds(struct mv_host_priv *hpriv,
 				      void __iomem *mmio)
 {
-	return;
+	void __iomem *hc_mmio;
+
+	hc_mmio = mv_hc_base(mmio, 0);
+
+	writel(SOC_LED_CTRL_ENABLE |
+		SOC_LED_CTRL_ACT_PRESENCE,
+		hc_mmio + SOC_LED_CTRL);
 }
 
 static void mv_soc_read_preamp(struct mv_host_priv *hpriv, int idx,
-- 
2.54.0

Re: [PATCH v2] ata: sata_mv: enable SoC SATA LED presence indication

[Damien Le Moal]

On 2026/09/25 23:28, Carlos Junior wrote:
> Enable the SoC SATA LED controller in mv_soc_enable_leds().
> 
> Set the LED controller enable and device presence multiplexing bits
> in the SoC SATA host-controller register. This enables SATA device
> presence indication on boards using the SoC SATA controller.
> 
> Signed-off-by: Carlos Junior <cjuniorfox@gmail.com>
> ---
> Changes in v2:
> - Do not initialize the SoC SATA LED blink bit; leave it to the
>   existing NCQ LED blink handling.
> 
>  drivers/ata/sata_mv.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
> index d260f04e6..52e33485b 100644
> --- a/drivers/ata/sata_mv.c
> +++ b/drivers/ata/sata_mv.c
> @@ -252,6 +252,7 @@ enum {
>  	SOC_LED_CTRL_BLINK	= (1 << 0),	/* Active LED blink */
>  	SOC_LED_CTRL_ACT_PRESENCE = (1 << 2),	/* Multiplex dev presence */
>  						/*  with dev activity LED */
> +	SOC_LED_CTRL_ENABLE	= (1 << 3),	/* Enable SATA LED control */
>  
>  	/* Shadow block registers */
>  	SHD_BLK			= 0x100,
> @@ -3382,12 +3383,20 @@ static void mv6_phy_errata(struct mv_host_priv *hpriv, void __iomem *mmio,
>  	writel(m2, port_mmio + PHY_MODE2);
>  }
>  
> -/* TODO: use the generic LED interface to configure the SATA Presence */
> -/* & Acitivy LEDs on the board */
> +/*
> + * Enable the SoC SATA LED controller and device presence
> + * indication on the board's SATA LEDs.
> + */
>  static void mv_soc_enable_leds(struct mv_host_priv *hpriv,
>  				      void __iomem *mmio)
>  {
> -	return;
> +	void __iomem *hc_mmio;
> +
> +	hc_mmio = mv_hc_base(mmio, 0);

This can be done in a single line with the declaration of the variable:

	void __iomem *hc_mmio = mv_hc_base(mmio, 0);

> +
> +	writel(SOC_LED_CTRL_ENABLE |
> +		SOC_LED_CTRL_ACT_PRESENCE,

No need to have this on 2 lines:

	writel(SOC_LED_CTRL_ENABLE | SOC_LED_CTRL_ACT_PRESENCE,
	       hc_mmio + SOC_LED_CTRL);

is nicer. And you can also get rid of the local variable:

	writel(SOC_LED_CTRL_ENABLE | SOC_LED_CTRL_ACT_PRESENCE,
	       mv_hc_base(mmio, 0) + SOC_LED_CTRL);

But if you prefer keeping the variable for readability, that's fine too.

With these nits fixed, feel free to add:

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>

> +		hc_mmio + SOC_LED_CTRL);
>  }
>  
>  static void mv_soc_read_preamp(struct mv_host_priv *hpriv, int idx,


-- 
Damien Le Moal
Western Digital Research

[PATCH v3] ata: sata_mv: enable SoC SATA LED presence indication

[Carlos Junior]

Enable the SoC SATA LED controller in mv_soc_enable_leds().

Set the LED controller enable and device presence multiplexing bits
in the SoC SATA host-controller register. This enables SATA device
presence indication on boards using the SoC SATA controller.

Signed-off-by: Carlos Junior <cjuniorfox@gmail.com>
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
---
Changes in v3:
- Simplify the writel() as suggested by Damien Le Moal.
- Add Reviewed-by tag from Damien Le Moal.

Changes in v2:
- Do not initialize the SoC SATA LED blink bit; leave it to the
  existing NCQ LED blink handling.
---
 drivers/ata/sata_mv.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
index d260f04e6..c408d04d9 100644
--- a/drivers/ata/sata_mv.c
+++ b/drivers/ata/sata_mv.c
@@ -252,6 +252,7 @@ enum {
 	SOC_LED_CTRL_BLINK	= (1 << 0),	/* Active LED blink */
 	SOC_LED_CTRL_ACT_PRESENCE = (1 << 2),	/* Multiplex dev presence */
 						/*  with dev activity LED */
+	SOC_LED_CTRL_ENABLE	= (1 << 3),	/* Enable SATA LED control */
 
 	/* Shadow block registers */
 	SHD_BLK			= 0x100,
@@ -3382,12 +3383,15 @@ static void mv6_phy_errata(struct mv_host_priv *hpriv, void __iomem *mmio,
 	writel(m2, port_mmio + PHY_MODE2);
 }
 
-/* TODO: use the generic LED interface to configure the SATA Presence */
-/* & Acitivy LEDs on the board */
+/*
+ * Enable the SoC SATA LED controller and device presence
+ * indication on the board's SATA LEDs.
+ */
 static void mv_soc_enable_leds(struct mv_host_priv *hpriv,
 				      void __iomem *mmio)
 {
-	return;
+	writel(SOC_LED_CTRL_ENABLE | SOC_LED_CTRL_ACT_PRESENCE,
+	       mv_hc_base(mmio, 0) + SOC_LED_CTRL);
 }
 
 static void mv_soc_read_preamp(struct mv_host_priv *hpriv, int idx,
-- 
2.54.0

Re: [PATCH v3] ata: sata_mv: enable SoC SATA LED presence indication

[Niklas Cassel]

On Mon, 28 Sep 2026 10:28:18 -0300, Carlos Junior wrote:
> Enable the SoC SATA LED controller in mv_soc_enable_leds().
> 
> Set the LED controller enable and device presence multiplexing bits
> in the SoC SATA host-controller register. This enables SATA device
> presence indication on boards using the SoC SATA controller.
> 
> 
> [...]

Applied to libata/linux.git (for-7.4), thanks!

[1/1] ata: sata_mv: enable SoC SATA LED presence indication
      https://git.kernel.org/libata/linux/c/f658c7bd

Kind regards,
Niklas