[alsa-devel] [PATCH 1/2] ASoC: Intel: sst: Fallback to BYT-CR if IRQ 5 is missing
Pierre-Louis Bossart
pierre-louis.bossart at linux.intel.com
Mon Dec 31 21:44:58 CET 2018
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 at gerhold.net>
>>> ---
>>> 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 at alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
More information about the Alsa-devel
mailing list