linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/3] ata: ahci_brcm: Follow-up changes for BCM7216
@ 2020-01-07 18:30 Florian Fainelli
  2020-01-07 18:30 ` [PATCH v3 1/3] ata: ahci_brcm: Correct reset control API usage Florian Fainelli
                   ` (4 more replies)
  0 siblings, 5 replies; 8+ messages in thread
From: Florian Fainelli @ 2020-01-07 18:30 UTC (permalink / raw)
  To: linux-kernel
  Cc: bcm-kernel-feedback-list, Florian Fainelli, Jens Axboe,
	Rob Herring, Mark Rutland, Hans de Goede, Philipp Zabel,
	Tejun Heo, Jaedon Shin,
	open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers),
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

Hi Jens, Philipp,

These three patches are a follow-up to my previous series titled: ata:
ahci_brcm: Fixes and new device support.

After submitting the BCM7216 RESCAL reset driver, Philipp the reset
controller maintained indicated that the reset line should be self
de-asserting and so reset_control_reset() should be used instead.

These three patches update the driver in that regard. It would be great if
you could apply those and get them queued up for 5.6 since they are
directly related to the previous series.

Changes in v3:
- introduced a preliminary patch making use of the proper reset control
  API in order to manage the optional reset controller line
- updated patches after introducing that preliminary patch

Changes in v2:
- updated error path after moving the reset line control

Thanks!

Florian Fainelli (3):
  ata: ahci_brcm: Correct reset control API usage
  ata: ahci_brcm: Perform reset after obtaining resources
  ata: ahci_brcm: BCM7216 reset is self de-asserting

 drivers/ata/ahci_brcm.c | 42 +++++++++++++++++++++++++----------------
 1 file changed, 26 insertions(+), 16 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v3 1/3] ata: ahci_brcm: Correct reset control API usage
  2020-01-07 18:30 [PATCH v3 0/3] ata: ahci_brcm: Follow-up changes for BCM7216 Florian Fainelli
@ 2020-01-07 18:30 ` Florian Fainelli
  2020-01-07 18:30 ` [PATCH v3 2/3] ata: ahci_brcm: Perform reset after obtaining resources Florian Fainelli
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Florian Fainelli @ 2020-01-07 18:30 UTC (permalink / raw)
  To: linux-kernel
  Cc: bcm-kernel-feedback-list, Florian Fainelli, Jens Axboe,
	Rob Herring, Mark Rutland, Hans de Goede, Philipp Zabel,
	Tejun Heo, Jaedon Shin,
	open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers),
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

Philipp provided suggestions on using the
devm_reset_control_get_optional_*() API such that checks against a NULL
or error reset control resource could be eliminated. In the process,
make sure that we also grab the BCM7216 reset control line using the
shared semantics, since the "rescal" reset fits that model.

Suggested-by: Philipp Zabel <p.zabel@pengutronix.de>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/ata/ahci_brcm.c | 32 +++++++++++++++++---------------
 1 file changed, 17 insertions(+), 15 deletions(-)

diff --git a/drivers/ata/ahci_brcm.c b/drivers/ata/ahci_brcm.c
index 13ceca687104..718516fe6997 100644
--- a/drivers/ata/ahci_brcm.c
+++ b/drivers/ata/ahci_brcm.c
@@ -349,11 +349,10 @@ static int brcm_ahci_suspend(struct device *dev)
 	brcm_sata_phys_disable(priv);
 
 	ret = ahci_platform_suspend(dev);
+	if (ret)
+		return ret;
 
-	if (!IS_ERR_OR_NULL(priv->rcdev))
-		reset_control_assert(priv->rcdev);
-
-	return ret;
+	return reset_control_assert(priv->rcdev);
 }
 
 static int brcm_ahci_resume(struct device *dev)
@@ -363,8 +362,7 @@ static int brcm_ahci_resume(struct device *dev)
 	struct brcm_ahci_priv *priv = hpriv->plat_data;
 	int ret = 0;
 
-	if (!IS_ERR_OR_NULL(priv->rcdev))
-		ret = reset_control_deassert(priv->rcdev);
+	ret = reset_control_deassert(priv->rcdev);
 	if (ret)
 		return ret;
 
@@ -425,7 +423,6 @@ static int brcm_ahci_probe(struct platform_device *pdev)
 {
 	const struct of_device_id *of_id;
 	struct device *dev = &pdev->dev;
-	const char *reset_name = NULL;
 	struct brcm_ahci_priv *priv;
 	struct ahci_host_priv *hpriv;
 	struct resource *res;
@@ -447,15 +444,21 @@ static int brcm_ahci_probe(struct platform_device *pdev)
 	if (IS_ERR(priv->top_ctrl))
 		return PTR_ERR(priv->top_ctrl);
 
-	/* Reset is optional depending on platform and named differently */
+	/* Reset is optional depending on platform and named differently
+	 * and of different kind (shared vs. exclusive)
+	 */
 	if (priv->version == BRCM_SATA_BCM7216)
-		reset_name = "rescal";
+		priv->rcdev = devm_reset_control_get_optional_shared(dev,
+								     "rescal");
 	else
-		reset_name = "ahci";
+		priv->rcdev = devm_reset_control_get_optional_exclusive(dev,
+									"ahci");
+	if (IS_ERR(priv->rcdev))
+		return PTR_ERR(priv->rcdev);
 
-	priv->rcdev = devm_reset_control_get(&pdev->dev, reset_name);
-	if (!IS_ERR_OR_NULL(priv->rcdev))
-		reset_control_deassert(priv->rcdev);
+	ret = reset_control_deassert(priv->rcdev);
+	if (ret)
+		return ret;
 
 	hpriv = ahci_platform_get_resources(pdev, 0);
 	if (IS_ERR(hpriv)) {
@@ -519,8 +522,7 @@ static int brcm_ahci_probe(struct platform_device *pdev)
 out_disable_clks:
 	ahci_platform_disable_clks(hpriv);
 out_reset:
-	if (!IS_ERR_OR_NULL(priv->rcdev))
-		reset_control_assert(priv->rcdev);
+	reset_control_assert(priv->rcdev);
 	return ret;
 }
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v3 2/3] ata: ahci_brcm: Perform reset after obtaining resources
  2020-01-07 18:30 [PATCH v3 0/3] ata: ahci_brcm: Follow-up changes for BCM7216 Florian Fainelli
  2020-01-07 18:30 ` [PATCH v3 1/3] ata: ahci_brcm: Correct reset control API usage Florian Fainelli
@ 2020-01-07 18:30 ` Florian Fainelli
  2020-01-07 18:30 ` [PATCH v3 3/3] ata: ahci_brcm: BCM7216 reset is self de-asserting Florian Fainelli
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Florian Fainelli @ 2020-01-07 18:30 UTC (permalink / raw)
  To: linux-kernel
  Cc: bcm-kernel-feedback-list, Florian Fainelli, Jens Axboe,
	Rob Herring, Mark Rutland, Hans de Goede, Philipp Zabel,
	Tejun Heo, Jaedon Shin,
	open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers),
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

Resources such as clocks, PHYs, regulators are likely to get a probe
deferral return code, which could lead to the AHCI controller being
reset a few times until it gets successfully probed. Since this is
typically the most time consuming operation, move it after the resources
have been acquired.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/ata/ahci_brcm.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/ata/ahci_brcm.c b/drivers/ata/ahci_brcm.c
index 718516fe6997..c229fea39a47 100644
--- a/drivers/ata/ahci_brcm.c
+++ b/drivers/ata/ahci_brcm.c
@@ -456,15 +456,9 @@ static int brcm_ahci_probe(struct platform_device *pdev)
 	if (IS_ERR(priv->rcdev))
 		return PTR_ERR(priv->rcdev);
 
-	ret = reset_control_deassert(priv->rcdev);
-	if (ret)
-		return ret;
-
 	hpriv = ahci_platform_get_resources(pdev, 0);
-	if (IS_ERR(hpriv)) {
-		ret = PTR_ERR(hpriv);
-		goto out_reset;
-	}
+	if (IS_ERR(hpriv))
+		return PTR_ERR(hpriv);
 
 	hpriv->plat_data = priv;
 	hpriv->flags = AHCI_HFLAG_WAKE_BEFORE_STOP | AHCI_HFLAG_NO_WRITE_TO_RO;
@@ -481,6 +475,10 @@ static int brcm_ahci_probe(struct platform_device *pdev)
 		break;
 	}
 
+	ret = reset_control_deassert(priv->rcdev);
+	if (ret)
+		return ret;
+
 	ret = ahci_platform_enable_clks(hpriv);
 	if (ret)
 		goto out_reset;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v3 3/3] ata: ahci_brcm: BCM7216 reset is self de-asserting
  2020-01-07 18:30 [PATCH v3 0/3] ata: ahci_brcm: Follow-up changes for BCM7216 Florian Fainelli
  2020-01-07 18:30 ` [PATCH v3 1/3] ata: ahci_brcm: Correct reset control API usage Florian Fainelli
  2020-01-07 18:30 ` [PATCH v3 2/3] ata: ahci_brcm: Perform reset after obtaining resources Florian Fainelli
@ 2020-01-07 18:30 ` Florian Fainelli
  2020-01-07 18:47 ` [PATCH v3 0/3] ata: ahci_brcm: Follow-up changes for BCM7216 Hans de Goede
  2020-01-08  9:25 ` Philipp Zabel
  4 siblings, 0 replies; 8+ messages in thread
From: Florian Fainelli @ 2020-01-07 18:30 UTC (permalink / raw)
  To: linux-kernel
  Cc: bcm-kernel-feedback-list, Florian Fainelli, Jens Axboe,
	Rob Herring, Mark Rutland, Hans de Goede, Philipp Zabel,
	Tejun Heo, Jaedon Shin,
	open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers),
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

The BCM7216 reset controller line is self-deasserting, unlike other
platforms, so make use of reset_control_reset() instead of
reset_control_deassert().

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/ata/ahci_brcm.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/ata/ahci_brcm.c b/drivers/ata/ahci_brcm.c
index c229fea39a47..62c948e56beb 100644
--- a/drivers/ata/ahci_brcm.c
+++ b/drivers/ata/ahci_brcm.c
@@ -352,7 +352,10 @@ static int brcm_ahci_suspend(struct device *dev)
 	if (ret)
 		return ret;
 
-	return reset_control_assert(priv->rcdev);
+	if (priv->version != BRCM_SATA_BCM7216)
+		ret = reset_control_assert(priv->rcdev);
+
+	return ret;
 }
 
 static int brcm_ahci_resume(struct device *dev)
@@ -362,7 +365,10 @@ static int brcm_ahci_resume(struct device *dev)
 	struct brcm_ahci_priv *priv = hpriv->plat_data;
 	int ret = 0;
 
-	ret = reset_control_deassert(priv->rcdev);
+	if (priv->version == BRCM_SATA_BCM7216)
+		ret = reset_control_reset(priv->rcdev);
+	else
+		ret = reset_control_deassert(priv->rcdev);
 	if (ret)
 		return ret;
 
@@ -475,7 +481,10 @@ static int brcm_ahci_probe(struct platform_device *pdev)
 		break;
 	}
 
-	ret = reset_control_deassert(priv->rcdev);
+	if (priv->version == BRCM_SATA_BCM7216)
+		ret = reset_control_reset(priv->rcdev);
+	else
+		ret = reset_control_deassert(priv->rcdev);
 	if (ret)
 		return ret;
 
@@ -520,7 +529,8 @@ static int brcm_ahci_probe(struct platform_device *pdev)
 out_disable_clks:
 	ahci_platform_disable_clks(hpriv);
 out_reset:
-	reset_control_assert(priv->rcdev);
+	if (priv->version != BRCM_SATA_BCM7216)
+		reset_control_assert(priv->rcdev);
 	return ret;
 }
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 0/3] ata: ahci_brcm: Follow-up changes for BCM7216
  2020-01-07 18:30 [PATCH v3 0/3] ata: ahci_brcm: Follow-up changes for BCM7216 Florian Fainelli
                   ` (2 preceding siblings ...)
  2020-01-07 18:30 ` [PATCH v3 3/3] ata: ahci_brcm: BCM7216 reset is self de-asserting Florian Fainelli
@ 2020-01-07 18:47 ` Hans de Goede
  2020-01-08  9:25 ` Philipp Zabel
  4 siblings, 0 replies; 8+ messages in thread
From: Hans de Goede @ 2020-01-07 18:47 UTC (permalink / raw)
  To: Florian Fainelli, linux-kernel
  Cc: bcm-kernel-feedback-list, Jens Axboe, Rob Herring, Mark Rutland,
	Philipp Zabel, Tejun Heo, Jaedon Shin,
	open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers),
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

Hi,

On 07-01-2020 19:30, Florian Fainelli wrote:
> Hi Jens, Philipp,
> 
> These three patches are a follow-up to my previous series titled: ata:
> ahci_brcm: Fixes and new device support.
> 
> After submitting the BCM7216 RESCAL reset driver, Philipp the reset
> controller maintained indicated that the reset line should be self
> de-asserting and so reset_control_reset() should be used instead.
> 
> These three patches update the driver in that regard. It would be great if
> you could apply those and get them queued up for 5.6 since they are
> directly related to the previous series.
> 
> Changes in v3:
> - introduced a preliminary patch making use of the proper reset control
>    API in order to manage the optional reset controller line
> - updated patches after introducing that preliminary patch
> 
> Changes in v2:
> - updated error path after moving the reset line control

Series looks good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans




> 
> Thanks!
> 
> Florian Fainelli (3):
>    ata: ahci_brcm: Correct reset control API usage
>    ata: ahci_brcm: Perform reset after obtaining resources
>    ata: ahci_brcm: BCM7216 reset is self de-asserting
> 
>   drivers/ata/ahci_brcm.c | 42 +++++++++++++++++++++++++----------------
>   1 file changed, 26 insertions(+), 16 deletions(-)
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 0/3] ata: ahci_brcm: Follow-up changes for BCM7216
  2020-01-07 18:30 [PATCH v3 0/3] ata: ahci_brcm: Follow-up changes for BCM7216 Florian Fainelli
                   ` (3 preceding siblings ...)
  2020-01-07 18:47 ` [PATCH v3 0/3] ata: ahci_brcm: Follow-up changes for BCM7216 Hans de Goede
@ 2020-01-08  9:25 ` Philipp Zabel
  2020-01-17  4:44   ` Florian Fainelli
  4 siblings, 1 reply; 8+ messages in thread
From: Philipp Zabel @ 2020-01-08  9:25 UTC (permalink / raw)
  To: Florian Fainelli, linux-kernel
  Cc: bcm-kernel-feedback-list, Jens Axboe, Rob Herring, Mark Rutland,
	Hans de Goede, Tejun Heo, Jaedon Shin,
	open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers),
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

Hi Florian,

On Tue, 2020-01-07 at 10:30 -0800, Florian Fainelli wrote:
> Hi Jens, Philipp,
> 
> These three patches are a follow-up to my previous series titled: ata:
> ahci_brcm: Fixes and new device support.
> 
> After submitting the BCM7216 RESCAL reset driver, Philipp the reset
> controller maintained indicated that the reset line should be self
> de-asserting and so reset_control_reset() should be used instead.
> 
> These three patches update the driver in that regard. It would be great if
> you could apply those and get them queued up for 5.6 since they are
> directly related to the previous series.
> 
> Changes in v3:
> - introduced a preliminary patch making use of the proper reset control
>   API in order to manage the optional reset controller line
> - updated patches after introducing that preliminary patch

The third patch could be simplified by storing the rescal reset control
in a separate struct member and relying on the optional reset control
API more. This is just a suggestion though, the series looks fine as-is.

Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>

regards
Philipp


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 0/3] ata: ahci_brcm: Follow-up changes for BCM7216
  2020-01-08  9:25 ` Philipp Zabel
@ 2020-01-17  4:44   ` Florian Fainelli
  2020-01-17  4:51     ` Jens Axboe
  0 siblings, 1 reply; 8+ messages in thread
From: Florian Fainelli @ 2020-01-17  4:44 UTC (permalink / raw)
  To: Philipp Zabel, Florian Fainelli, linux-kernel, Jens Axboe
  Cc: bcm-kernel-feedback-list, Rob Herring, Mark Rutland,
	Hans de Goede, Tejun Heo, Jaedon Shin,
	open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers),
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS



On 1/8/2020 1:25 AM, Philipp Zabel wrote:
> Hi Florian,
> 
> On Tue, 2020-01-07 at 10:30 -0800, Florian Fainelli wrote:
>> Hi Jens, Philipp,
>>
>> These three patches are a follow-up to my previous series titled: ata:
>> ahci_brcm: Fixes and new device support.
>>
>> After submitting the BCM7216 RESCAL reset driver, Philipp the reset
>> controller maintained indicated that the reset line should be self
>> de-asserting and so reset_control_reset() should be used instead.
>>
>> These three patches update the driver in that regard. It would be great if
>> you could apply those and get them queued up for 5.6 since they are
>> directly related to the previous series.
>>
>> Changes in v3:
>> - introduced a preliminary patch making use of the proper reset control
>>   API in order to manage the optional reset controller line
>> - updated patches after introducing that preliminary patch
> 
> The third patch could be simplified by storing the rescal reset control
> in a separate struct member and relying on the optional reset control
> API more. This is just a suggestion though, the series looks fine as-is.
> 
> Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>

Thanks! Jens is that good for you?
-- 
Florian

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 0/3] ata: ahci_brcm: Follow-up changes for BCM7216
  2020-01-17  4:44   ` Florian Fainelli
@ 2020-01-17  4:51     ` Jens Axboe
  0 siblings, 0 replies; 8+ messages in thread
From: Jens Axboe @ 2020-01-17  4:51 UTC (permalink / raw)
  To: Florian Fainelli, Philipp Zabel, Florian Fainelli, linux-kernel
  Cc: bcm-kernel-feedback-list, Rob Herring, Mark Rutland,
	Hans de Goede, Tejun Heo, Jaedon Shin,
	open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers),
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

On 1/16/20 9:44 PM, Florian Fainelli wrote:
> 
> 
> On 1/8/2020 1:25 AM, Philipp Zabel wrote:
>> Hi Florian,
>>
>> On Tue, 2020-01-07 at 10:30 -0800, Florian Fainelli wrote:
>>> Hi Jens, Philipp,
>>>
>>> These three patches are a follow-up to my previous series titled: ata:
>>> ahci_brcm: Fixes and new device support.
>>>
>>> After submitting the BCM7216 RESCAL reset driver, Philipp the reset
>>> controller maintained indicated that the reset line should be self
>>> de-asserting and so reset_control_reset() should be used instead.
>>>
>>> These three patches update the driver in that regard. It would be great if
>>> you could apply those and get them queued up for 5.6 since they are
>>> directly related to the previous series.
>>>
>>> Changes in v3:
>>> - introduced a preliminary patch making use of the proper reset control
>>>   API in order to manage the optional reset controller line
>>> - updated patches after introducing that preliminary patch
>>
>> The third patch could be simplified by storing the rescal reset control
>> in a separate struct member and relying on the optional reset control
>> API more. This is just a suggestion though, the series looks fine as-is.
>>
>> Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
> 
> Thanks! Jens is that good for you?

Can you re-send against the current branch and include the reviewed-bys
from here as well?


-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-01-17  4:51 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-07 18:30 [PATCH v3 0/3] ata: ahci_brcm: Follow-up changes for BCM7216 Florian Fainelli
2020-01-07 18:30 ` [PATCH v3 1/3] ata: ahci_brcm: Correct reset control API usage Florian Fainelli
2020-01-07 18:30 ` [PATCH v3 2/3] ata: ahci_brcm: Perform reset after obtaining resources Florian Fainelli
2020-01-07 18:30 ` [PATCH v3 3/3] ata: ahci_brcm: BCM7216 reset is self de-asserting Florian Fainelli
2020-01-07 18:47 ` [PATCH v3 0/3] ata: ahci_brcm: Follow-up changes for BCM7216 Hans de Goede
2020-01-08  9:25 ` Philipp Zabel
2020-01-17  4:44   ` Florian Fainelli
2020-01-17  4:51     ` Jens Axboe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).