mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH] mci: add new MCI_BROKEN_CD option for testing
@ 2022-07-20  4:15 Ahmad Fatoum
  2022-07-20  8:23 ` Marco Felsch
  0 siblings, 1 reply; 3+ messages in thread
From: Ahmad Fatoum @ 2022-07-20  4:15 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

In remote labs co-located with other hardware, we've observed card
detect levers of different boards to sporadically fail to detect
the card, e.g. because the cable on the usbsdmux was yanked around
by accident. When this happens, barebox usually boots up normally as
the card detect is ignored and then Linux waits indefinitely for
the card-detect to turn active. Add a new config option that can be
enabled to avoid these issues altogether.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/mci/Kconfig    | 15 +++++++++++++++
 drivers/mci/mci-core.c | 35 +++++++++++++++++++++++++++++++----
 2 files changed, 46 insertions(+), 4 deletions(-)

diff --git a/drivers/mci/Kconfig b/drivers/mci/Kconfig
index 21d53c0c3f0b..651e59259790 100644
--- a/drivers/mci/Kconfig
+++ b/drivers/mci/Kconfig
@@ -56,6 +56,21 @@ config MCI_MMC_GPP_PARTITIONS
 	  Note: by default, 'MMC' devices have no 'general purpose partitions',
 	  it requires a special one-time configuration step to enable them.
 
+config MCI_BROKEN_CD
+	bool "ignore card-detect pin on boot and in OS"
+	help
+	  Say 'y' here to have barebox unconditionally ignore the
+	  card-detect pin for its own operation and manipulate the
+	  kernel DT, so all detected MCI cards are polled instead
+	  of expecting the card detect lever to behave correctly.
+	  If you need more fine grained control use of_property
+	  in an init script:
+
+	    of_property -fd mmc0 cd-gpios
+	    of_property -fs mmc0 broken-cd
+
+	  If unsure, say 'n' here.
+
 comment "--- MCI host drivers ---"
 
 config MCI_DW
diff --git a/drivers/mci/mci-core.c b/drivers/mci/mci-core.c
index b8f71e15986e..6018391e1abb 100644
--- a/drivers/mci/mci-core.c
+++ b/drivers/mci/mci-core.c
@@ -1739,6 +1739,27 @@ static int mci_register_partition(struct mci_part *part)
 	return 0;
 }
 
+static int of_broken_cd_fixup(struct device_node *root, void *ctx)
+{
+	struct device_d *hw_dev = ctx;
+	struct device_node *np;
+	char *name;
+
+	name = of_get_reproducible_name(hw_dev->device_node);
+	np = of_find_node_by_reproducible_name(root, name);
+	free(name);
+	if (!np) {
+		dev_warn(hw_dev, "Cannot find nodepath %s, cannot fixup\n",
+			 hw_dev->device_node->full_name);
+		return -EINVAL;
+	}
+
+	of_property_write_bool(np, "cd-gpios", false);
+	of_property_write_bool(np, "broken-cd", true);
+
+	return 0;
+}
+
 /**
  * Probe an MCI card at the given host interface
  * @param mci MCI device instance
@@ -1750,10 +1771,13 @@ static int mci_card_probe(struct mci *mci)
 	int i, rc, disknum, ret;
 	bool has_bootpart = false;
 
-	if (host->card_present && !host->card_present(host) &&
-	    !host->non_removable) {
-		dev_err(&mci->dev, "no card inserted\n");
-		return -ENODEV;
+	if (host->card_present && !host->card_present(host) && !host->non_removable) {
+		if (IS_ENABLED(CONFIG_MCI_BROKEN_CD)) {
+			dev_info(&mci->dev, "no card inserted (ignoring)\n");
+		} else {
+			dev_err(&mci->dev, "no card inserted\n");
+			return -ENODEV;
+		}
 	}
 
 	ret = regulator_enable(host->supply);
@@ -1839,6 +1863,9 @@ static int mci_card_probe(struct mci *mci)
 					   &mci->boot_ack_enable, mci);
 	}
 
+	if (IS_ENABLED(CONFIG_MCI_BROKEN_CD) && !host->no_sd && dev_of_node(host->hw_dev))
+		return of_register_fixup(of_broken_cd_fixup, host->hw_dev);
+
 	dev_dbg(&mci->dev, "SD Card successfully added\n");
 
 on_error:
-- 
2.30.2




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

* Re: [PATCH] mci: add new MCI_BROKEN_CD option for testing
  2022-07-20  4:15 [PATCH] mci: add new MCI_BROKEN_CD option for testing Ahmad Fatoum
@ 2022-07-20  8:23 ` Marco Felsch
  2022-07-20  8:58   ` Ahmad Fatoum
  0 siblings, 1 reply; 3+ messages in thread
From: Marco Felsch @ 2022-07-20  8:23 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: barebox

Hi Ahmad,

On 22-07-20, Ahmad Fatoum wrote:
> In remote labs co-located with other hardware, we've observed card
> detect levers of different boards to sporadically fail to detect
> the card, e.g. because the cable on the usbsdmux was yanked around
> by accident. When this happens, barebox usually boots up normally as
> the card detect is ignored and then Linux waits indefinitely for
> the card-detect to turn active. Add a new config option that can be
> enabled to avoid these issues altogether.

what do you think about to have a device property instead of a compile
time depend behaviour? One use case could be a field return customer
board and the customer didn't enabled this Kconfig option since he had
no issues. This board can't be reprogrammed because it is secure locked.
It would be cool to have your live-patching on such boards.

Regards,
  Marco

> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  drivers/mci/Kconfig    | 15 +++++++++++++++
>  drivers/mci/mci-core.c | 35 +++++++++++++++++++++++++++++++----
>  2 files changed, 46 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mci/Kconfig b/drivers/mci/Kconfig
> index 21d53c0c3f0b..651e59259790 100644
> --- a/drivers/mci/Kconfig
> +++ b/drivers/mci/Kconfig
> @@ -56,6 +56,21 @@ config MCI_MMC_GPP_PARTITIONS
>  	  Note: by default, 'MMC' devices have no 'general purpose partitions',
>  	  it requires a special one-time configuration step to enable them.
>  
> +config MCI_BROKEN_CD
> +	bool "ignore card-detect pin on boot and in OS"
> +	help
> +	  Say 'y' here to have barebox unconditionally ignore the
> +	  card-detect pin for its own operation and manipulate the
> +	  kernel DT, so all detected MCI cards are polled instead
> +	  of expecting the card detect lever to behave correctly.
> +	  If you need more fine grained control use of_property
> +	  in an init script:
> +
> +	    of_property -fd mmc0 cd-gpios
> +	    of_property -fs mmc0 broken-cd
> +
> +	  If unsure, say 'n' here.
> +
>  comment "--- MCI host drivers ---"
>  
>  config MCI_DW
> diff --git a/drivers/mci/mci-core.c b/drivers/mci/mci-core.c
> index b8f71e15986e..6018391e1abb 100644
> --- a/drivers/mci/mci-core.c
> +++ b/drivers/mci/mci-core.c
> @@ -1739,6 +1739,27 @@ static int mci_register_partition(struct mci_part *part)
>  	return 0;
>  }
>  
> +static int of_broken_cd_fixup(struct device_node *root, void *ctx)
> +{
> +	struct device_d *hw_dev = ctx;
> +	struct device_node *np;
> +	char *name;
> +
> +	name = of_get_reproducible_name(hw_dev->device_node);
> +	np = of_find_node_by_reproducible_name(root, name);
> +	free(name);
> +	if (!np) {
> +		dev_warn(hw_dev, "Cannot find nodepath %s, cannot fixup\n",
> +			 hw_dev->device_node->full_name);
> +		return -EINVAL;
> +	}
> +
> +	of_property_write_bool(np, "cd-gpios", false);
> +	of_property_write_bool(np, "broken-cd", true);
> +
> +	return 0;
> +}
> +
>  /**
>   * Probe an MCI card at the given host interface
>   * @param mci MCI device instance
> @@ -1750,10 +1771,13 @@ static int mci_card_probe(struct mci *mci)
>  	int i, rc, disknum, ret;
>  	bool has_bootpart = false;
>  
> -	if (host->card_present && !host->card_present(host) &&
> -	    !host->non_removable) {
> -		dev_err(&mci->dev, "no card inserted\n");
> -		return -ENODEV;
> +	if (host->card_present && !host->card_present(host) && !host->non_removable) {
> +		if (IS_ENABLED(CONFIG_MCI_BROKEN_CD)) {
> +			dev_info(&mci->dev, "no card inserted (ignoring)\n");
> +		} else {
> +			dev_err(&mci->dev, "no card inserted\n");
> +			return -ENODEV;
> +		}
>  	}
>  
>  	ret = regulator_enable(host->supply);
> @@ -1839,6 +1863,9 @@ static int mci_card_probe(struct mci *mci)
>  					   &mci->boot_ack_enable, mci);
>  	}
>  
> +	if (IS_ENABLED(CONFIG_MCI_BROKEN_CD) && !host->no_sd && dev_of_node(host->hw_dev))
> +		return of_register_fixup(of_broken_cd_fixup, host->hw_dev);
> +
>  	dev_dbg(&mci->dev, "SD Card successfully added\n");
>  
>  on_error:
> -- 
> 2.30.2
> 
> 
> 



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

* Re: [PATCH] mci: add new MCI_BROKEN_CD option for testing
  2022-07-20  8:23 ` Marco Felsch
@ 2022-07-20  8:58   ` Ahmad Fatoum
  0 siblings, 0 replies; 3+ messages in thread
From: Ahmad Fatoum @ 2022-07-20  8:58 UTC (permalink / raw)
  To: Marco Felsch; +Cc: barebox

Hello Marco,

On 20.07.22 10:23, Marco Felsch wrote:
> Hi Ahmad,
> 
> On 22-07-20, Ahmad Fatoum wrote:
>> In remote labs co-located with other hardware, we've observed card
>> detect levers of different boards to sporadically fail to detect
>> the card, e.g. because the cable on the usbsdmux was yanked around
>> by accident. When this happens, barebox usually boots up normally as
>> the card detect is ignored and then Linux waits indefinitely for
>> the card-detect to turn active. Add a new config option that can be
>> enabled to avoid these issues altogether.
> 
> what do you think about to have a device property instead of a compile
> time depend behaviour? One use case could be a field return customer
> board and the customer didn't enabled this Kconfig option since he had
> no issues. This board can't be reprogrammed because it is secure locked.
> It would be cool to have your live-patching on such boards.

I thought about making it a device parameter, but decided against it.
It's IMO a very specific use case that most don't care for and that
can be scripted if needed, so no need to bloat all configurations
that use a SD card.

As for your particular example, just bend the lever, so it always touches
ground or solder it so it has no other choice. :)

Cheers,
Ahmad


> 
> Regards,
>   Marco
> 
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>>  drivers/mci/Kconfig    | 15 +++++++++++++++
>>  drivers/mci/mci-core.c | 35 +++++++++++++++++++++++++++++++----
>>  2 files changed, 46 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/mci/Kconfig b/drivers/mci/Kconfig
>> index 21d53c0c3f0b..651e59259790 100644
>> --- a/drivers/mci/Kconfig
>> +++ b/drivers/mci/Kconfig
>> @@ -56,6 +56,21 @@ config MCI_MMC_GPP_PARTITIONS
>>  	  Note: by default, 'MMC' devices have no 'general purpose partitions',
>>  	  it requires a special one-time configuration step to enable them.
>>  
>> +config MCI_BROKEN_CD
>> +	bool "ignore card-detect pin on boot and in OS"
>> +	help
>> +	  Say 'y' here to have barebox unconditionally ignore the
>> +	  card-detect pin for its own operation and manipulate the
>> +	  kernel DT, so all detected MCI cards are polled instead
>> +	  of expecting the card detect lever to behave correctly.
>> +	  If you need more fine grained control use of_property
>> +	  in an init script:
>> +
>> +	    of_property -fd mmc0 cd-gpios
>> +	    of_property -fs mmc0 broken-cd
>> +
>> +	  If unsure, say 'n' here.
>> +
>>  comment "--- MCI host drivers ---"
>>  
>>  config MCI_DW
>> diff --git a/drivers/mci/mci-core.c b/drivers/mci/mci-core.c
>> index b8f71e15986e..6018391e1abb 100644
>> --- a/drivers/mci/mci-core.c
>> +++ b/drivers/mci/mci-core.c
>> @@ -1739,6 +1739,27 @@ static int mci_register_partition(struct mci_part *part)
>>  	return 0;
>>  }
>>  
>> +static int of_broken_cd_fixup(struct device_node *root, void *ctx)
>> +{
>> +	struct device_d *hw_dev = ctx;
>> +	struct device_node *np;
>> +	char *name;
>> +
>> +	name = of_get_reproducible_name(hw_dev->device_node);
>> +	np = of_find_node_by_reproducible_name(root, name);
>> +	free(name);
>> +	if (!np) {
>> +		dev_warn(hw_dev, "Cannot find nodepath %s, cannot fixup\n",
>> +			 hw_dev->device_node->full_name);
>> +		return -EINVAL;
>> +	}
>> +
>> +	of_property_write_bool(np, "cd-gpios", false);
>> +	of_property_write_bool(np, "broken-cd", true);
>> +
>> +	return 0;
>> +}
>> +
>>  /**
>>   * Probe an MCI card at the given host interface
>>   * @param mci MCI device instance
>> @@ -1750,10 +1771,13 @@ static int mci_card_probe(struct mci *mci)
>>  	int i, rc, disknum, ret;
>>  	bool has_bootpart = false;
>>  
>> -	if (host->card_present && !host->card_present(host) &&
>> -	    !host->non_removable) {
>> -		dev_err(&mci->dev, "no card inserted\n");
>> -		return -ENODEV;
>> +	if (host->card_present && !host->card_present(host) && !host->non_removable) {
>> +		if (IS_ENABLED(CONFIG_MCI_BROKEN_CD)) {
>> +			dev_info(&mci->dev, "no card inserted (ignoring)\n");
>> +		} else {
>> +			dev_err(&mci->dev, "no card inserted\n");
>> +			return -ENODEV;
>> +		}
>>  	}
>>  
>>  	ret = regulator_enable(host->supply);
>> @@ -1839,6 +1863,9 @@ static int mci_card_probe(struct mci *mci)
>>  					   &mci->boot_ack_enable, mci);
>>  	}
>>  
>> +	if (IS_ENABLED(CONFIG_MCI_BROKEN_CD) && !host->no_sd && dev_of_node(host->hw_dev))
>> +		return of_register_fixup(of_broken_cd_fixup, host->hw_dev);
>> +
>>  	dev_dbg(&mci->dev, "SD Card successfully added\n");
>>  
>>  on_error:
>> -- 
>> 2.30.2
>>
>>
>>
> 


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |



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

end of thread, other threads:[~2022-07-20  9:01 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-20  4:15 [PATCH] mci: add new MCI_BROKEN_CD option for testing Ahmad Fatoum
2022-07-20  8:23 ` Marco Felsch
2022-07-20  8:58   ` Ahmad Fatoum

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox