Hi,
On 05-12-16 14:23, Hans de Goede wrote:
Hi,
On 05-12-16 11:59, Thierry Reding wrote:
On Mon, Dec 05, 2016 at 09:18:03AM +0100, Hans de Goede wrote:
Hi,
On 05-12-16 08:46, Thierry Reding wrote:
On Fri, Dec 02, 2016 at 11:17:35AM +0100, Hans de Goede wrote:
The primary consumer of the lpss pwm is the i915 kms driver, but
currently that driver cannot get the pwm because i915 platforms are
not using devicetree and pwm-lpss does not call pwm_add_table.
Another problem is that i915 does not support get_pwm returning
-EPROBE_DEFER and i915's init is very complex and this is almost
impossible to fix.
This commit changes the PWM_LPSS Kconfig from a tristate to a bool, so
that when the i915 driver loads the lpss pwm will be available avoiding
the -EPROBE_DEFER issue. Note that this is identical to how the same
problem was solved for the pwm-crc driver.
Being builtin also allows calling pwm_add_table directly from the
pwm-lpss code, otherwise the pwm_add_table call would need to be put
somewhere else to ensure it happens before i915 calls pwm_get,
even if i915 would support -EPROBE_DEFER.
Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
---
drivers/pwm/Kconfig | 12 +++---------
drivers/pwm/pwm-lpss.c | 11 +++++++++++
2 files changed, 14 insertions(+), 9 deletions(-)
This is completely backwards. You're putting board-specific bits into a
generic driver.
This is not really board specific I'm advertising that the goal of
the pwm is to be used to control a backlight.
pwm_add_table() is a board-specific API. Documentation/pwm.txt says that
"board setup code" should be using pwm_add_table(). Using it from within
the provider is completely the opposite.
The problem here really is that there is no such thing as
board setup code on x86 + EFI/ACPI, that is supposedly all
handled by the EFI/ACPI code there.
Before typing this reply I've been thinking about another place
to put the pwm_add_table call put I cannot come up with any.
I suggested drivers/platform/x86. A bunch of code in there is doing
exactly the kind of board/platform setup stuff that you're trying to do
here.
All drivers under drivers/platform/x86 bind to something, be it
an ACPI interface or an actual platform device. In the case of the
pwm-lpss we have an actual platform or pci device and a driver binding
to it, that is the only common code path I see where I can add the
pwm_add_table.
Sure I can put a built-in bit of code under drivers/platform/x86
which checks from its module_init() that there is an pwm-lpss controller
present (either listed under ACPI or through PCI) and then calls
pwm_add_table, but seems silly. Note as said this then must be
built-in, because if it is a module nothing will trigger the
loading of the module, unless I add duplicate MODULE_DEVICE_TABLE
tables in there with the code under drivers/pwm/pwm-lpss.
TL;DR: the problem is that something needs to trigger / activate
the code doing the pwm_add_table() and AFAICT we have no other
trigger then the presence of the pwm-lpss device, at which point
the pwm_lpss_probe function becomes the best place to do the
pwm_add_table call.
ping ?
Can I please get an answer to the above. I'm happy to put the
pwm_add_table call somewhere-else if that is what it takes to
get these fixes merged, but I don't see any obvious other place
to put this. So can you please tell me where to put the
pwm_add_table call, if not in pwm-lpss.c ? Note as said before
it should be triggered by the acpi-ids used to bind
the pwm-lpss driver, which to me really makes the pwm-lpss
driver the best place to do it.
Regards,
Hans
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
