Hi Pierre,
On 05-02-21, 09:15, Pierre-Louis Bossart wrote:
Thanks for the review Vinod.
On 04-02-21, 14:48, Pierre-Louis Bossart wrote:
HP Spectre x360 Convertible devices expose invalid _ADR fields in the DSDT, which prevents codec drivers from probing. A possible solution is to override the DSDT, but that's just too painful for users.
This patch suggests a simple DMI-based quirk to remap the existing invalid ADR information into valid ones.
While I agree with the approach, I do not like the implementation. The quirks in firmware should not reside in core code. Drivers are the right place, of course we need to add callbacks into driver for this.
So I did a quick hacking and added the below patch, I think you can add the quirks in Intel driver based on DMI for this.
I thought about this, but the Intel driver is about the *master* configuration. It's not really about slave-related _ADR. If and when the IP configuration changes, it'll be problematic to have such quirks in the middle.
Okay, but ADR is not really a slave configuration either. I feel it is system wide description..
At the end of the day, the problem is an ACPI one, not an Intel master one, and I put the code where it's protected by CONFIG_ACPI.
Right, to be more specific it is a buggy firmware implementation and not following of the specs by device vendors.
I don't mind doing the change, but the notion of conflating Intel master and list of slave quirks isn't without its own problems.
Same is true for soundwire core (slave/slave-apci or whatever). The issue I have is that this will sure become big, so I would like this to be moved away from all soundwire core files. The right place IMO for this is respective drivers, intel/codec/machine please do take your pick. My attempt here was to move this to Intel driver here as I felt that was the right place for platform issues here.. do feel free to move any other place except core files...
An alternate solution would be to break the ACPI and OF slave initialization into two separate files (slave-acpi.c and slave-of.c), that way there is a cleaner split.
Or we put all those quirks in a dedicated slave-dmi-quirks.c and use your solution. That may be more manageable since the list of quirks is historically likely to grow.
It's really ugly in all cases.
I try to look at the positive side of things: if we have quirks to handle it's an indicator that more platforms are moving to SoundWire...
I hope though that it doesn't reach the level of Baytrail where most of the machine driver is a dictionary of quirks.
Hope that it wont, but having seen things, I think it may come to it!