Re: [PATCH 1/2] ASoC: Intel: sst: Fallback to BYT-CR if IRQ 5 is missing

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 12/31/18 10:30 AM, Stephan Gerhold wrote:
On Mon, Dec 31, 2018 at 09:38:21AM -0600, Pierre-Louis Bossart wrote:
On 12/22/18 8:47 AM, Stephan Gerhold wrote:
Some devices detected as BYT-T by the PMIC-type based detection
have only a single IRQ listed in the 80860F28 ACPI device. This
causes -ENXIO later when attempting to get the IRQ at index 5.
It turns out these devices behave more like BYT-CR devices,
and using the IRQ at index 0 makes sound work correctly.

This patch adds a fallback for these devices to is_byt_cr():
If there is no IRQ resource at index 5, treating the device
as BYT-T is guaranteed to fail later, so we can safely treat
these devices as BYT-CR without breaking any working device.

Link: http://mailman.alsa-project.org/pipermail/alsa-devel/2018-December/143176.html
Signed-off-by: Stephan Gerhold <stephan@xxxxxxxxxxx>
---
Moved the "Detected Baytrail-CR platform" message to is_byt_cr()
so we can log a different message if the fallback is used.

Tested this on my device as-is, and simulated a "normal"
BYT-T and BYT-CR device (copied their IRQs to a custom DSDT).

   sound/soc/intel/atom/sst/sst_acpi.c | 24 ++++++++++++++++++------
   1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/sound/soc/intel/atom/sst/sst_acpi.c b/sound/soc/intel/atom/sst/sst_acpi.c
index 3a95ebbfc45d..755a396121ff 100644
--- a/sound/soc/intel/atom/sst/sst_acpi.c
+++ b/sound/soc/intel/atom/sst/sst_acpi.c
@@ -255,10 +255,22 @@ static int is_byt(void)
   	return status;
   }
-static int is_byt_cr(struct device *dev, bool *bytcr)
+static int is_byt_cr(struct platform_device *pdev, bool *bytcr)
   {
+	struct device *dev = &pdev->dev;
   	int status = 0;
+	if (platform_get_resource(pdev, IORESOURCE_IRQ, 5) == NULL) {
+		/*
+		 * Some devices detected as BYT-T have only a single IRQ listed,
+		 * causing platform_get_irq with index 5 to return -ENXIO.
+		 * The correct IRQ in this case is at index 0, as used on BYT-CR.
+		 */
+		dev_info(dev, "Falling back to Baytrail-CR platform\n");
+		*bytcr = true;
+		return status;
+	}
+
Isn't this going to bypass the PMIC-based detection on all BYT-CR devices?
Maybe move this code as a fallback used when the PMIC-based detection isn't
positive?

Except for the message that is logged, it does not really make a
difference. PMIC-based detection is still used for most BYT-CR devices,
which usually have 6 IRQs listed. For the few that have not, the end
result (bytcr = true) is the same, even if they now use the fallback.

I mentioned this in a previous mail when I asked you which option you
would prefer (see [1]). Since is_byt_cr() has multiple returns,
I cannot just put it last without refactoring the entire method.
(Which is something I wanted to avoid...)

Ah yes, but there was a side thread with Andy Shevchenko where we discussed that the initial return can be simplified since there are wrappers for iosf_mbi_available even when CONFIG_IOSF_MBI is not enabled. The code could be something like

diff --git a/sound/soc/intel/atom/sst/sst_acpi.c b/sound/soc/intel/atom/sst/sst_acpi.c
index ac542535b9d5..58e389a64c6a 100644
--- a/sound/soc/intel/atom/sst/sst_acpi.c
+++ b/sound/soc/intel/atom/sst/sst_acpi.c
@@ -255,17 +255,16 @@ static int is_byt(void)
        return status;
 }

-static int is_byt_cr(struct device *dev, bool *bytcr)
+static int is_byt_cr(struct platform_device *pdev, bool *bytcr)
 {
+       struct device *dev = &pdev->dev;
+       u32 bios_status;
        int status = 0;

-       if (IS_ENABLED(CONFIG_IOSF_MBI)) {
-               u32 bios_status;
+       if (!is_byt())
+               return status;

-               if (!is_byt() || !iosf_mbi_available()) {
-                       /* bail silently */
-                       return status;
-               }
+       if (iosf_mbi_available()) {

                status = iosf_mbi_read(BT_MBI_UNIT_PMC, /* 0x04 PUNIT */
                                       MBI_REG_READ, /* 0x10 */
@@ -286,6 +285,20 @@ static int is_byt_cr(struct device *dev, bool *bytcr)
        } else {
                dev_info(dev, "IOSF_MBI not enabled, no BYT-CR detection\n");
        }
+
+       if (*bytcr == false &&
+           platform_get_resource(pdev, IORESOURCE_IRQ, 5) == NULL) {
+               /*
+                * Some devices detected as BYT-T have only a single IRQ listed,
+                * causing platform_get_irq with index 5 to return -ENXIO.
+                * The correct IRQ in this case is at index 0, as used on
+                * BYT-CR.
+                */
+               dev_info(dev, "Falling back to Baytrail-CR platform\n");
+               status = 0;
+               *bytcr = true;
+       }
+
        return status;
 }





[1]: http://mailman.alsa-project.org/pipermail/alsa-devel/2018-December/143339.html

   	if (IS_ENABLED(CONFIG_IOSF_MBI)) {
   		u32 bios_status;
@@ -278,10 +290,12 @@ static int is_byt_cr(struct device *dev, bool *bytcr)
   			/* bits 26:27 mirror PMIC options */
   			bios_status = (bios_status >> 26) & 3;
-			if ((bios_status == 1) || (bios_status == 3))
+			if ((bios_status == 1) || (bios_status == 3)) {
+				dev_info(dev, "Detected Baytrail-CR platform\n");
   				*bytcr = true;
-			else
+			} else {
   				dev_info(dev, "BYT-CR not detected\n");
+			}
   		}
   	} else {
   		dev_info(dev, "IOSF_MBI not enabled, no BYT-CR detection\n");
@@ -333,10 +347,8 @@ static int sst_acpi_probe(struct platform_device *pdev)
   	if (ret < 0)
   		return ret;
-	ret = is_byt_cr(dev, &bytcr);
+	ret = is_byt_cr(pdev, &bytcr);
   	if (!(ret < 0 || !bytcr)) {
-		dev_info(dev, "Detected Baytrail-CR platform\n");
-
   		/* override resource info */
   		byt_rvp_platform_data.res_info = &bytcr_res_info;
   	}
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel




[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux