* [PATCH] dt-bindings: input: syna,rmi4: Make "additionalProperties: true" explicit
From: Rob Herring @ 2023-09-26 14:42 UTC (permalink / raw)
To: Dmitry Torokhov, Krzysztof Kozlowski, Conor Dooley,
Jason A. Donenfeld, Matthias Schiffer, Vincent Huang
Cc: linux-input, devicetree, linux-kernel
Make it explicit that the not yet documented child nodes have additional
properties and the child node schema is not complete.
Signed-off-by: Rob Herring <robh@kernel.org>
---
Documentation/devicetree/bindings/input/syna,rmi4.yaml | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/input/syna,rmi4.yaml b/Documentation/devicetree/bindings/input/syna,rmi4.yaml
index 4d4e1a8e36be..b522c8d3ce0d 100644
--- a/Documentation/devicetree/bindings/input/syna,rmi4.yaml
+++ b/Documentation/devicetree/bindings/input/syna,rmi4.yaml
@@ -164,6 +164,8 @@ patternProperties:
"^rmi4-f[0-9a-f]+@[0-9a-f]+$":
type: object
+ additionalProperties: true
+
description:
Other functions, not documented yet.
--
2.40.1
^ permalink raw reply related
* Re: [PATCH 13/15] platform/x86/amd/pmf: Add PMF-AMDGPU set interface
From: Shyam Sundar S K @ 2023-09-26 13:48 UTC (permalink / raw)
To: Christian König, Hans de Goede, markgross, basavaraj.natikar,
jikos, benjamin.tissoires, alexander.deucher, Xinhui.Pan, airlied,
daniel
Cc: amd-gfx, platform-driver-x86, dri-devel, Patil.Reddy, linux-input,
mario.limonciello
In-Reply-To: <4e79121f-01bb-729b-1e70-043e8911cb12@amd.com>
Hi Christian,
On 9/26/2023 6:47 PM, Christian König wrote:
> Am 26.09.23 um 14:56 schrieb Hans de Goede:
>> Hi,
>>
>> On 9/26/23 13:24, Shyam Sundar S K wrote:
>>> Hi Hans,
>>>
>>> On 9/26/2023 4:05 PM, Hans de Goede wrote:
>>>> Hi,
>>>>
>>>> On 9/22/23 19:50, Shyam Sundar S K wrote:
>>>>> For the Smart PC Solution to fully work, it has to enact to the
>>>>> actions
>>>>> coming from TA. Add the initial code path for set interface to
>>>>> AMDGPU.
>>>>>
>>>>> Co-developed-by: Mario Limonciello <mario.limonciello@amd.com>
>>>>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>>>>> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
>>>>> ---
>>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c | 21 +++++++++++++++++++++
>>>>> drivers/platform/x86/amd/pmf/pmf.h | 2 ++
>>>>> drivers/platform/x86/amd/pmf/tee-if.c | 19 +++++++++++++++++--
>>>>> include/linux/amd-pmf-io.h | 1 +
>>>>> 4 files changed, 41 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
>>>>> index 232d11833ddc..5c567bff0548 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
>>>>> @@ -68,3 +68,24 @@ int amd_pmf_get_gfx_data(struct
>>>>> amd_gpu_pmf_data *pmf)
>>>>> return 0;
>>>>> }
>>>>> EXPORT_SYMBOL_GPL(amd_pmf_get_gfx_data);
>>>>> +
>>>>> +int amd_pmf_set_gfx_data(struct amd_gpu_pmf_data *pmf)
>>>>> +{
>>>>> + struct drm_device *drm_dev = pci_get_drvdata(pmf->gpu_dev);
>>>>> + struct amdgpu_device *adev = drm_to_adev(drm_dev);
>>>>> + struct backlight_device *bd;
>>>>> +
>>>>> + if (!(adev->flags & AMD_IS_APU)) {
>>>>> + DRM_ERROR("PMF-AMDGPU interface not supported\n");
>>>>> + return -ENODEV;
>>>>> + }
>>>>> +
>>>>> + bd = backlight_device_get_by_type(BACKLIGHT_RAW);
>>>>> + if (!bd)
>>>>> + return -ENODEV;
>>>> This assumes that the backlight is always controller by the amdgpu's
>>>> native backlight driver, but it might e.g. also be handled by
>>>> eacpi-video or by nvidia_wmi_ec_backlight (when using an AMD APU +
>>>> nvidia dgpu).
>>> PMF is meant for AMD APUs(atleast for now) and the _HID will only be
>>> made visible if its AMD laptop. So using amdgpu's native BACKLIGHT_RAW
>>> should be safe, right?
>> Users can pass say acpi_backlight=video and use the acpi_video
>> driver for backlight control instead of the native GPU backlight
>> control.
>>
>>>> For now what should be done here is to call
>>>> acpi_video_get_backlight_type()
>>>> and then translate the return value from this into a backlight-type:
>>>>
>>>> acpi_backlight_video -> BACKLIGHT_FIRMWARE
>>>> acpi_backlight_vendor, -> BACKLIGHT_PLATFORM
>>>> acpi_backlight_native, -> BACKLIGHT_RAW
>>>> acpi_backlight_nvidia_wmi_ec, -> BACKLIGHT_FIRMWARE
>>>> acpi_backlight_apple_gmux, -> BACKLIGHT_PLATFORM
>>>>
>>> I can add this change in the v2, do you insist on this?
>> Insist is a strong word, but I think that it is a good idea to have
>> this. Evenutally it looks like this code will need to either
>> integrate with
>> the drm drivers lot more; or the drm core needs to export some special
>> hooks for this which the PMF code can then call.
>>
>> Actually thinking more about this, I think that the right thing to do
>> here is make some code register brightness control as a cooling device
>> (which I think is already done in some cases) and then have the PMF
>> code use the cooling-device APIs for this.
>>
>> IMHO that would be a much cleaner solution then this hack.
>
> Yeah, fully agree with Hans. This looks like a rather extreme hack to me.
Sure. Let me think in this direction.
>
> Apart from that what exactly is this thing supposed to do? Prevent
> overheating by reducing the brightness?
Yes that can be one of the cases. But the thought process here is to
help OEMs build their own policies.
A policy is combination of inputs and outputs. Inputs are the
conditions that has to be met and outputs are the actions to be set/done.
The output actions come from PMF TA. One example policy apart from the
case you mentioned is:
if ambient light (received from amd_sfh) is low ; reduce the screen
brightness (received from amdgpu) or vice versa.
Thanks,
Shyam
>
> Regards,
> Christian.
>
>>
>> Regards,
>>
>> Hans
>>
>>
>>
>>> Thanks,
>>> Shyam
>>>
>>>> Also I'm worried about probe order here, this code currently assumes
>>>> that the GPU or other backlight driver has loaded before this runs,
>>>> which is not necessarily the case.
>>>>
>>>> I think that if the backlight_device_get_by_type() fails this
>>>> should be retried say every 10 seconds from some delayed workqueue
>>>> for at least a couple of minutes after boot.
>>>>
>>>> Regards,
>>>>
>>>> Hans
>>>>
>>>>
>>>>
>>>>
>>>>> +
>>>>> + backlight_device_set_brightness(bd, pmf->brightness);
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>> +EXPORT_SYMBOL_GPL(amd_pmf_set_gfx_data);
>>>>> diff --git a/drivers/platform/x86/amd/pmf/pmf.h
>>>>> b/drivers/platform/x86/amd/pmf/pmf.h
>>>>> index 9032df4ba48a..ce89cc0daa5a 100644
>>>>> --- a/drivers/platform/x86/amd/pmf/pmf.h
>>>>> +++ b/drivers/platform/x86/amd/pmf/pmf.h
>>>>> @@ -73,6 +73,7 @@
>>>>> #define PMF_POLICY_STT_SKINTEMP_APU 7
>>>>> #define PMF_POLICY_STT_SKINTEMP_HS2 8
>>>>> #define PMF_POLICY_SYSTEM_STATE 9
>>>>> +#define PMF_POLICY_DISPLAY_BRIGHTNESS 12
>>>>> #define PMF_POLICY_P3T 38
>>>>> /* TA macros */
>>>>> @@ -480,6 +481,7 @@ enum ta_pmf_error_type {
>>>>> };
>>>>> struct pmf_action_table {
>>>>> + unsigned long display_brightness;
>>>>> enum system_state system_state;
>>>>> unsigned long spl; /* in mW */
>>>>> unsigned long sppt; /* in mW */
>>>>> diff --git a/drivers/platform/x86/amd/pmf/tee-if.c
>>>>> b/drivers/platform/x86/amd/pmf/tee-if.c
>>>>> index 1608996654e8..eefffff83a4c 100644
>>>>> --- a/drivers/platform/x86/amd/pmf/tee-if.c
>>>>> +++ b/drivers/platform/x86/amd/pmf/tee-if.c
>>>>> @@ -79,10 +79,10 @@ static int amd_pmf_update_uevents(struct
>>>>> amd_pmf_dev *dev, u16 event)
>>>>> return 0;
>>>>> }
>>>>> -static void amd_pmf_apply_policies(struct amd_pmf_dev *dev,
>>>>> struct ta_pmf_enact_result *out)
>>>>> +static int amd_pmf_apply_policies(struct amd_pmf_dev *dev,
>>>>> struct ta_pmf_enact_result *out)
>>>>> {
>>>>> u32 val, event = 0;
>>>>> - int idx;
>>>>> + int idx, ret;
>>>>> for (idx = 0; idx < out->actions_count; idx++) {
>>>>> val = out->actions_list[idx].value;
>>>>> @@ -160,8 +160,23 @@ static void amd_pmf_apply_policies(struct
>>>>> amd_pmf_dev *dev, struct ta_pmf_enact_
>>>>> dev->prev_data->system_state = 0;
>>>>> }
>>>>> break;
>>>>> +
>>>>> + case PMF_POLICY_DISPLAY_BRIGHTNESS:
>>>>> + ret = amd_pmf_get_gfx_data(&dev->gfx_data);
>>>>> + if (ret)
>>>>> + return ret;
>>>>> +
>>>>> + dev->prev_data->display_brightness =
>>>>> dev->gfx_data.brightness;
>>>>> + if (dev->prev_data->display_brightness != val) {
>>>>> + dev->gfx_data.brightness = val;
>>>>> + amd_pmf_set_gfx_data(&dev->gfx_data);
>>>>> + dev_dbg(dev->dev, "update DISPLAY_BRIGHTNESS :
>>>>> %d\n", val);
>>>>> + }
>>>>> + break;
>>>>> }
>>>>> }
>>>>> +
>>>>> + return 0;
>>>>> }
>>>>> static int amd_pmf_invoke_cmd_enact(struct amd_pmf_dev *dev)
>>>>> diff --git a/include/linux/amd-pmf-io.h b/include/linux/amd-pmf-io.h
>>>>> index a2d4af231362..ecae387ddaa6 100644
>>>>> --- a/include/linux/amd-pmf-io.h
>>>>> +++ b/include/linux/amd-pmf-io.h
>>>>> @@ -25,4 +25,5 @@ struct amd_gpu_pmf_data {
>>>>> };
>>>>> int amd_pmf_get_gfx_data(struct amd_gpu_pmf_data *pmf);
>>>>> +int amd_pmf_set_gfx_data(struct amd_gpu_pmf_data *pmf);
>>>>> #endif
>
^ permalink raw reply
* Re: [PATCH 13/15] platform/x86/amd/pmf: Add PMF-AMDGPU set interface
From: Christian König @ 2023-09-26 13:17 UTC (permalink / raw)
To: Hans de Goede, Shyam Sundar S K, markgross, basavaraj.natikar,
jikos, benjamin.tissoires, alexander.deucher, christian.koenig,
Xinhui.Pan, airlied, daniel
Cc: amd-gfx, platform-driver-x86, dri-devel, Patil.Reddy, linux-input,
mario.limonciello
In-Reply-To: <6c047e17-66b0-c3ff-1b4e-4478663619a4@redhat.com>
Am 26.09.23 um 14:56 schrieb Hans de Goede:
> Hi,
>
> On 9/26/23 13:24, Shyam Sundar S K wrote:
>> Hi Hans,
>>
>> On 9/26/2023 4:05 PM, Hans de Goede wrote:
>>> Hi,
>>>
>>> On 9/22/23 19:50, Shyam Sundar S K wrote:
>>>> For the Smart PC Solution to fully work, it has to enact to the actions
>>>> coming from TA. Add the initial code path for set interface to AMDGPU.
>>>>
>>>> Co-developed-by: Mario Limonciello <mario.limonciello@amd.com>
>>>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>>>> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
>>>> ---
>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c | 21 +++++++++++++++++++++
>>>> drivers/platform/x86/amd/pmf/pmf.h | 2 ++
>>>> drivers/platform/x86/amd/pmf/tee-if.c | 19 +++++++++++++++++--
>>>> include/linux/amd-pmf-io.h | 1 +
>>>> 4 files changed, 41 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
>>>> index 232d11833ddc..5c567bff0548 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
>>>> @@ -68,3 +68,24 @@ int amd_pmf_get_gfx_data(struct amd_gpu_pmf_data *pmf)
>>>> return 0;
>>>> }
>>>> EXPORT_SYMBOL_GPL(amd_pmf_get_gfx_data);
>>>> +
>>>> +int amd_pmf_set_gfx_data(struct amd_gpu_pmf_data *pmf)
>>>> +{
>>>> + struct drm_device *drm_dev = pci_get_drvdata(pmf->gpu_dev);
>>>> + struct amdgpu_device *adev = drm_to_adev(drm_dev);
>>>> + struct backlight_device *bd;
>>>> +
>>>> + if (!(adev->flags & AMD_IS_APU)) {
>>>> + DRM_ERROR("PMF-AMDGPU interface not supported\n");
>>>> + return -ENODEV;
>>>> + }
>>>> +
>>>> + bd = backlight_device_get_by_type(BACKLIGHT_RAW);
>>>> + if (!bd)
>>>> + return -ENODEV;
>>> This assumes that the backlight is always controller by the amdgpu's
>>> native backlight driver, but it might e.g. also be handled by
>>> eacpi-video or by nvidia_wmi_ec_backlight (when using an AMD APU +
>>> nvidia dgpu).
>> PMF is meant for AMD APUs(atleast for now) and the _HID will only be
>> made visible if its AMD laptop. So using amdgpu's native BACKLIGHT_RAW
>> should be safe, right?
> Users can pass say acpi_backlight=video and use the acpi_video
> driver for backlight control instead of the native GPU backlight
> control.
>
>>> For now what should be done here is to call acpi_video_get_backlight_type()
>>> and then translate the return value from this into a backlight-type:
>>>
>>> acpi_backlight_video -> BACKLIGHT_FIRMWARE
>>> acpi_backlight_vendor, -> BACKLIGHT_PLATFORM
>>> acpi_backlight_native, -> BACKLIGHT_RAW
>>> acpi_backlight_nvidia_wmi_ec, -> BACKLIGHT_FIRMWARE
>>> acpi_backlight_apple_gmux, -> BACKLIGHT_PLATFORM
>>>
>> I can add this change in the v2, do you insist on this?
> Insist is a strong word, but I think that it is a good idea to have
> this. Evenutally it looks like this code will need to either integrate with
> the drm drivers lot more; or the drm core needs to export some special
> hooks for this which the PMF code can then call.
>
> Actually thinking more about this, I think that the right thing to do
> here is make some code register brightness control as a cooling device
> (which I think is already done in some cases) and then have the PMF
> code use the cooling-device APIs for this.
>
> IMHO that would be a much cleaner solution then this hack.
Yeah, fully agree with Hans. This looks like a rather extreme hack to me.
Apart from that what exactly is this thing supposed to do? Prevent
overheating by reducing the brightness?
Regards,
Christian.
>
> Regards,
>
> Hans
>
>
>
>> Thanks,
>> Shyam
>>
>>> Also I'm worried about probe order here, this code currently assumes
>>> that the GPU or other backlight driver has loaded before this runs,
>>> which is not necessarily the case.
>>>
>>> I think that if the backlight_device_get_by_type() fails this
>>> should be retried say every 10 seconds from some delayed workqueue
>>> for at least a couple of minutes after boot.
>>>
>>> Regards,
>>>
>>> Hans
>>>
>>>
>>>
>>>
>>>> +
>>>> + backlight_device_set_brightness(bd, pmf->brightness);
>>>> +
>>>> + return 0;
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(amd_pmf_set_gfx_data);
>>>> diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
>>>> index 9032df4ba48a..ce89cc0daa5a 100644
>>>> --- a/drivers/platform/x86/amd/pmf/pmf.h
>>>> +++ b/drivers/platform/x86/amd/pmf/pmf.h
>>>> @@ -73,6 +73,7 @@
>>>> #define PMF_POLICY_STT_SKINTEMP_APU 7
>>>> #define PMF_POLICY_STT_SKINTEMP_HS2 8
>>>> #define PMF_POLICY_SYSTEM_STATE 9
>>>> +#define PMF_POLICY_DISPLAY_BRIGHTNESS 12
>>>> #define PMF_POLICY_P3T 38
>>>>
>>>> /* TA macros */
>>>> @@ -480,6 +481,7 @@ enum ta_pmf_error_type {
>>>> };
>>>>
>>>> struct pmf_action_table {
>>>> + unsigned long display_brightness;
>>>> enum system_state system_state;
>>>> unsigned long spl; /* in mW */
>>>> unsigned long sppt; /* in mW */
>>>> diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
>>>> index 1608996654e8..eefffff83a4c 100644
>>>> --- a/drivers/platform/x86/amd/pmf/tee-if.c
>>>> +++ b/drivers/platform/x86/amd/pmf/tee-if.c
>>>> @@ -79,10 +79,10 @@ static int amd_pmf_update_uevents(struct amd_pmf_dev *dev, u16 event)
>>>> return 0;
>>>> }
>>>>
>>>> -static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_result *out)
>>>> +static int amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_result *out)
>>>> {
>>>> u32 val, event = 0;
>>>> - int idx;
>>>> + int idx, ret;
>>>>
>>>> for (idx = 0; idx < out->actions_count; idx++) {
>>>> val = out->actions_list[idx].value;
>>>> @@ -160,8 +160,23 @@ static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_
>>>> dev->prev_data->system_state = 0;
>>>> }
>>>> break;
>>>> +
>>>> + case PMF_POLICY_DISPLAY_BRIGHTNESS:
>>>> + ret = amd_pmf_get_gfx_data(&dev->gfx_data);
>>>> + if (ret)
>>>> + return ret;
>>>> +
>>>> + dev->prev_data->display_brightness = dev->gfx_data.brightness;
>>>> + if (dev->prev_data->display_brightness != val) {
>>>> + dev->gfx_data.brightness = val;
>>>> + amd_pmf_set_gfx_data(&dev->gfx_data);
>>>> + dev_dbg(dev->dev, "update DISPLAY_BRIGHTNESS : %d\n", val);
>>>> + }
>>>> + break;
>>>> }
>>>> }
>>>> +
>>>> + return 0;
>>>> }
>>>>
>>>> static int amd_pmf_invoke_cmd_enact(struct amd_pmf_dev *dev)
>>>> diff --git a/include/linux/amd-pmf-io.h b/include/linux/amd-pmf-io.h
>>>> index a2d4af231362..ecae387ddaa6 100644
>>>> --- a/include/linux/amd-pmf-io.h
>>>> +++ b/include/linux/amd-pmf-io.h
>>>> @@ -25,4 +25,5 @@ struct amd_gpu_pmf_data {
>>>> };
>>>>
>>>> int amd_pmf_get_gfx_data(struct amd_gpu_pmf_data *pmf);
>>>> +int amd_pmf_set_gfx_data(struct amd_gpu_pmf_data *pmf);
>>>> #endif
^ permalink raw reply
* Re: [PATCH 13/15] platform/x86/amd/pmf: Add PMF-AMDGPU set interface
From: Hans de Goede @ 2023-09-26 12:56 UTC (permalink / raw)
To: Shyam Sundar S K, markgross, basavaraj.natikar, jikos,
benjamin.tissoires, alexander.deucher, christian.koenig,
Xinhui.Pan, airlied, daniel
Cc: Patil.Reddy, mario.limonciello, platform-driver-x86, linux-input,
amd-gfx, dri-devel
In-Reply-To: <437fbd5c-386f-4609-a350-77f61c8aaa0c@amd.com>
Hi,
On 9/26/23 13:24, Shyam Sundar S K wrote:
> Hi Hans,
>
> On 9/26/2023 4:05 PM, Hans de Goede wrote:
>> Hi,
>>
>> On 9/22/23 19:50, Shyam Sundar S K wrote:
>>> For the Smart PC Solution to fully work, it has to enact to the actions
>>> coming from TA. Add the initial code path for set interface to AMDGPU.
>>>
>>> Co-developed-by: Mario Limonciello <mario.limonciello@amd.com>
>>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>>> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c | 21 +++++++++++++++++++++
>>> drivers/platform/x86/amd/pmf/pmf.h | 2 ++
>>> drivers/platform/x86/amd/pmf/tee-if.c | 19 +++++++++++++++++--
>>> include/linux/amd-pmf-io.h | 1 +
>>> 4 files changed, 41 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
>>> index 232d11833ddc..5c567bff0548 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
>>> @@ -68,3 +68,24 @@ int amd_pmf_get_gfx_data(struct amd_gpu_pmf_data *pmf)
>>> return 0;
>>> }
>>> EXPORT_SYMBOL_GPL(amd_pmf_get_gfx_data);
>>> +
>>> +int amd_pmf_set_gfx_data(struct amd_gpu_pmf_data *pmf)
>>> +{
>>> + struct drm_device *drm_dev = pci_get_drvdata(pmf->gpu_dev);
>>> + struct amdgpu_device *adev = drm_to_adev(drm_dev);
>>> + struct backlight_device *bd;
>>> +
>>> + if (!(adev->flags & AMD_IS_APU)) {
>>> + DRM_ERROR("PMF-AMDGPU interface not supported\n");
>>> + return -ENODEV;
>>> + }
>>> +
>>> + bd = backlight_device_get_by_type(BACKLIGHT_RAW);
>>> + if (!bd)
>>> + return -ENODEV;
>>
>> This assumes that the backlight is always controller by the amdgpu's
>> native backlight driver, but it might e.g. also be handled by
>> eacpi-video or by nvidia_wmi_ec_backlight (when using an AMD APU +
>> nvidia dgpu).
>
> PMF is meant for AMD APUs(atleast for now) and the _HID will only be
> made visible if its AMD laptop. So using amdgpu's native BACKLIGHT_RAW
> should be safe, right?
Users can pass say acpi_backlight=video and use the acpi_video
driver for backlight control instead of the native GPU backlight
control.
>
>>
>> For now what should be done here is to call acpi_video_get_backlight_type()
>> and then translate the return value from this into a backlight-type:
>>
>> acpi_backlight_video -> BACKLIGHT_FIRMWARE
>> acpi_backlight_vendor, -> BACKLIGHT_PLATFORM
>> acpi_backlight_native, -> BACKLIGHT_RAW
>> acpi_backlight_nvidia_wmi_ec, -> BACKLIGHT_FIRMWARE
>> acpi_backlight_apple_gmux, -> BACKLIGHT_PLATFORM
>>
>
> I can add this change in the v2, do you insist on this?
Insist is a strong word, but I think that it is a good idea to have
this. Evenutally it looks like this code will need to either integrate with
the drm drivers lot more; or the drm core needs to export some special
hooks for this which the PMF code can then call.
Actually thinking more about this, I think that the right thing to do
here is make some code register brightness control as a cooling device
(which I think is already done in some cases) and then have the PMF
code use the cooling-device APIs for this.
IMHO that would be a much cleaner solution then this hack.
Regards,
Hans
>
> Thanks,
> Shyam
>
>> Also I'm worried about probe order here, this code currently assumes
>> that the GPU or other backlight driver has loaded before this runs,
>> which is not necessarily the case.
>>
>> I think that if the backlight_device_get_by_type() fails this
>> should be retried say every 10 seconds from some delayed workqueue
>> for at least a couple of minutes after boot.
>>
>> Regards,
>>
>> Hans
>>
>>
>>
>>
>>> +
>>> + backlight_device_set_brightness(bd, pmf->brightness);
>>> +
>>> + return 0;
>>> +}
>>> +EXPORT_SYMBOL_GPL(amd_pmf_set_gfx_data);
>>> diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
>>> index 9032df4ba48a..ce89cc0daa5a 100644
>>> --- a/drivers/platform/x86/amd/pmf/pmf.h
>>> +++ b/drivers/platform/x86/amd/pmf/pmf.h
>>> @@ -73,6 +73,7 @@
>>> #define PMF_POLICY_STT_SKINTEMP_APU 7
>>> #define PMF_POLICY_STT_SKINTEMP_HS2 8
>>> #define PMF_POLICY_SYSTEM_STATE 9
>>> +#define PMF_POLICY_DISPLAY_BRIGHTNESS 12
>>> #define PMF_POLICY_P3T 38
>>>
>>> /* TA macros */
>>> @@ -480,6 +481,7 @@ enum ta_pmf_error_type {
>>> };
>>>
>>> struct pmf_action_table {
>>> + unsigned long display_brightness;
>>> enum system_state system_state;
>>> unsigned long spl; /* in mW */
>>> unsigned long sppt; /* in mW */
>>> diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
>>> index 1608996654e8..eefffff83a4c 100644
>>> --- a/drivers/platform/x86/amd/pmf/tee-if.c
>>> +++ b/drivers/platform/x86/amd/pmf/tee-if.c
>>> @@ -79,10 +79,10 @@ static int amd_pmf_update_uevents(struct amd_pmf_dev *dev, u16 event)
>>> return 0;
>>> }
>>>
>>> -static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_result *out)
>>> +static int amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_result *out)
>>> {
>>> u32 val, event = 0;
>>> - int idx;
>>> + int idx, ret;
>>>
>>> for (idx = 0; idx < out->actions_count; idx++) {
>>> val = out->actions_list[idx].value;
>>> @@ -160,8 +160,23 @@ static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_
>>> dev->prev_data->system_state = 0;
>>> }
>>> break;
>>> +
>>> + case PMF_POLICY_DISPLAY_BRIGHTNESS:
>>> + ret = amd_pmf_get_gfx_data(&dev->gfx_data);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + dev->prev_data->display_brightness = dev->gfx_data.brightness;
>>> + if (dev->prev_data->display_brightness != val) {
>>> + dev->gfx_data.brightness = val;
>>> + amd_pmf_set_gfx_data(&dev->gfx_data);
>>> + dev_dbg(dev->dev, "update DISPLAY_BRIGHTNESS : %d\n", val);
>>> + }
>>> + break;
>>> }
>>> }
>>> +
>>> + return 0;
>>> }
>>>
>>> static int amd_pmf_invoke_cmd_enact(struct amd_pmf_dev *dev)
>>> diff --git a/include/linux/amd-pmf-io.h b/include/linux/amd-pmf-io.h
>>> index a2d4af231362..ecae387ddaa6 100644
>>> --- a/include/linux/amd-pmf-io.h
>>> +++ b/include/linux/amd-pmf-io.h
>>> @@ -25,4 +25,5 @@ struct amd_gpu_pmf_data {
>>> };
>>>
>>> int amd_pmf_get_gfx_data(struct amd_gpu_pmf_data *pmf);
>>> +int amd_pmf_set_gfx_data(struct amd_gpu_pmf_data *pmf);
>>> #endif
>>
>
^ permalink raw reply
* Re: [PATCH V2 1/2] dt-bindings: input: Introduce Himax HID-over-SPI device
From: Conor Dooley @ 2023-09-26 12:53 UTC (permalink / raw)
To: yang tylor
Cc: Conor Dooley, dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt,
conor+dt, jikos, benjamin.tissoires, linux-input, devicetree,
linux-kernel, poyuan_chang, hbarnor, jingyliang@chromium.org,
wuxy23, luolm1, hung poyu
In-Reply-To: <CAGD2q_YBfDT950tyxEF87ZeiANgea_x8S16Ud5K2bcQ+eL9T=w@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 4994 bytes --]
On Tue, Sep 26, 2023 at 05:52:39PM +0800, yang tylor wrote:
> On Tue, Sep 26, 2023 at 5:02 PM Conor Dooley <conor@kernel.org> wrote:
> >
> > On Mon, Sep 25, 2023 at 06:16:29PM +0800, yang tylor wrote:
> > > On Mon, Sep 25, 2023 at 4:41 PM Conor Dooley <conor.dooley@microchip.com> wrote:
> > > >
> > > > On Mon, Sep 25, 2023 at 09:44:21AM +0800, yang tylor wrote:
> > > > > On Fri, Sep 22, 2023 at 11:31 PM Conor Dooley <conor@kernel.org> wrote:
> > > > > >
> > > > > > On Fri, Sep 22, 2023 at 05:43:54PM +0800, yang tylor wrote:
> > > > > > > On Fri, Sep 22, 2023 at 5:22 PM Conor Dooley <conor@kernel.org> wrote:
> > > > > > > >
> > > > > > > > On Fri, Sep 22, 2023 at 03:56:25PM +0800, yang tylor wrote:
> > > > > > > > > On Tue, Sep 19, 2023 at 7:09 PM Conor Dooley <conor@kernel.org> wrote:
> > > > > > > > > > On Tue, Sep 19, 2023 at 05:31:29PM +0800, yang tylor wrote:
> > > > > > > >
> > > > > > > > > > > The behavior of "himax,boot_time_fw_upgrade" seems not stable and
> > > > > > > > > > > should be removed. "himax,fw_in_flash", I use the kernel config for
> > > > > > > > > > > user to select.
> > > > > > > > > >
> > > > > > > > > > That seems like a bad idea, we want to be able to build one kernel that
> > > > > > > > > > works for all hardware at the same time.
> > > > > > > > > >
> > > > > > > > > I see, so I should take that back?
> > > > > > > > > I'll explain more about it.
> > > > > > > >
> > > > > > > > Are there particular ICs where the firmware would always be in flash and
> > > > > > > > others where it would never be? Or is this a choice made by the board or
> > > > > > > > system designer?
> > > > > > > >
> > > > > > > Most cases it's about the system designer's decision. But some ICs may be forced
> > > > > > > to use flash because of its architecture(multiple IC inside, need to
> > > > > > > load firmware to
> > > > > > > multiple IC's sram by master IC). But if there is no limitation on
> > > > > > > this part, most system
> > > > > > > designers will prefer flashless.
> > > > > >
> > > > > > Forgive me if I am not understanding correctly, there are some ICs that
> > > > > > will need to load the firmware from flash and there are some where it
> > > > > > will be a decision made by the designer of the board. Is the flash part
> > > > > > of the IC or is it an external flash chip?
> > > > > >
> > > > >
> > > > > Both are possible, it depends on the IC type. For TDDI, the IC is long
> > > > > and thin, placed on panel PCB, flash will be located at the external
> > > > > flash chip. For the OLED TP, IC is usually placed at FPC and its flash
> > > > > is embedded, thus the IC size is large compared to TDDI. But from the
> > > > > driver's perspective either external flash or embedded flash, the IC
> > > > > itself will load firmware from flash automatically when reset pin is
> > > > > released. Only if firmware is loading from the host storage system,
> > > > > the driver needs to operate the IC in detail.
> > > >
> > > >
> > > > Since there are ICs that can use the external flash or have it loaded
> > > > from the host, it sounds like you do need a property to differentiate
> > > > between those cases.
> > > Yep.
> > >
> > > > Is it sufficient to just set the firmware-name property for these cases?
> > > > If the property exists, then you know you need to load firmware & what
> > > > its name is. If it doesn't, then the firmware either isn't needed or
> > > > will be automatically loaded from the external flash.
> >
> > > We have a default prefix firmware name(like himax_xxxx.bin) in the driver code.
> >
> > How do you intend generating the name of the firmware file? I assume the
> > same firmware doesn't work on every IC, so you'll need to pick a
> > different one depending on the compatible?
> >
> If considering a firmware library line-up for all the incoming panels
> of this driver.
> We would use PID as part of the file name. Because all the support panels would
> have a unique PID associated. Which will make the firmware name like
> himax_xxx_{$PID}.bin. The problem is, we need to know PID before firmware load
> at no flash condition. Thus PID information is required in dts when
> no-flash-flag
> is specified.
Firstly, where does the "xxx" come from?
And you're making it sound more like having firmware-name is suitable
for this use case, given you need to determine the name of the file to
use based on something that is hardware specific but is not
dynamically detectable.
Thanks,
Conor.
> > > So we'll look for it when no-flash-flag is specified. In our experience,
> > > forcing a prefix firmware name helps the user to aware what firmware
> > > they are dealing with.
>
> If a more simple solution for no-flash condition is needed, as you mentioned,
> specifying a firmware name in dts would be the best. Otherwise, a
> no-flash-flag and
> PID information needs to be added in dts.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH 13/15] platform/x86/amd/pmf: Add PMF-AMDGPU set interface
From: Shyam Sundar S K @ 2023-09-26 11:24 UTC (permalink / raw)
To: Hans de Goede, markgross, basavaraj.natikar, jikos,
benjamin.tissoires, alexander.deucher, christian.koenig,
Xinhui.Pan, airlied, daniel
Cc: Patil.Reddy, mario.limonciello, platform-driver-x86, linux-input,
amd-gfx, dri-devel
In-Reply-To: <2e201a3b-d75f-916d-5135-b084ad5da23e@redhat.com>
Hi Hans,
On 9/26/2023 4:05 PM, Hans de Goede wrote:
> Hi,
>
> On 9/22/23 19:50, Shyam Sundar S K wrote:
>> For the Smart PC Solution to fully work, it has to enact to the actions
>> coming from TA. Add the initial code path for set interface to AMDGPU.
>>
>> Co-developed-by: Mario Limonciello <mario.limonciello@amd.com>
>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c | 21 +++++++++++++++++++++
>> drivers/platform/x86/amd/pmf/pmf.h | 2 ++
>> drivers/platform/x86/amd/pmf/tee-if.c | 19 +++++++++++++++++--
>> include/linux/amd-pmf-io.h | 1 +
>> 4 files changed, 41 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
>> index 232d11833ddc..5c567bff0548 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
>> @@ -68,3 +68,24 @@ int amd_pmf_get_gfx_data(struct amd_gpu_pmf_data *pmf)
>> return 0;
>> }
>> EXPORT_SYMBOL_GPL(amd_pmf_get_gfx_data);
>> +
>> +int amd_pmf_set_gfx_data(struct amd_gpu_pmf_data *pmf)
>> +{
>> + struct drm_device *drm_dev = pci_get_drvdata(pmf->gpu_dev);
>> + struct amdgpu_device *adev = drm_to_adev(drm_dev);
>> + struct backlight_device *bd;
>> +
>> + if (!(adev->flags & AMD_IS_APU)) {
>> + DRM_ERROR("PMF-AMDGPU interface not supported\n");
>> + return -ENODEV;
>> + }
>> +
>> + bd = backlight_device_get_by_type(BACKLIGHT_RAW);
>> + if (!bd)
>> + return -ENODEV;
>
> This assumes that the backlight is always controller by the amdgpu's
> native backlight driver, but it might e.g. also be handled by
> eacpi-video or by nvidia_wmi_ec_backlight (when using an AMD APU +
> nvidia dgpu).
PMF is meant for AMD APUs(atleast for now) and the _HID will only be
made visible if its AMD laptop. So using amdgpu's native BACKLIGHT_RAW
should be safe, right?
>
> For now what should be done here is to call acpi_video_get_backlight_type()
> and then translate the return value from this into a backlight-type:
>
> acpi_backlight_video -> BACKLIGHT_FIRMWARE
> acpi_backlight_vendor, -> BACKLIGHT_PLATFORM
> acpi_backlight_native, -> BACKLIGHT_RAW
> acpi_backlight_nvidia_wmi_ec, -> BACKLIGHT_FIRMWARE
> acpi_backlight_apple_gmux, -> BACKLIGHT_PLATFORM
>
I can add this change in the v2, do you insist on this?
Thanks,
Shyam
> Also I'm worried about probe order here, this code currently assumes
> that the GPU or other backlight driver has loaded before this runs,
> which is not necessarily the case.
>
> I think that if the backlight_device_get_by_type() fails this
> should be retried say every 10 seconds from some delayed workqueue
> for at least a couple of minutes after boot.
>
> Regards,
>
> Hans
>
>
>
>
>> +
>> + backlight_device_set_brightness(bd, pmf->brightness);
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(amd_pmf_set_gfx_data);
>> diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
>> index 9032df4ba48a..ce89cc0daa5a 100644
>> --- a/drivers/platform/x86/amd/pmf/pmf.h
>> +++ b/drivers/platform/x86/amd/pmf/pmf.h
>> @@ -73,6 +73,7 @@
>> #define PMF_POLICY_STT_SKINTEMP_APU 7
>> #define PMF_POLICY_STT_SKINTEMP_HS2 8
>> #define PMF_POLICY_SYSTEM_STATE 9
>> +#define PMF_POLICY_DISPLAY_BRIGHTNESS 12
>> #define PMF_POLICY_P3T 38
>>
>> /* TA macros */
>> @@ -480,6 +481,7 @@ enum ta_pmf_error_type {
>> };
>>
>> struct pmf_action_table {
>> + unsigned long display_brightness;
>> enum system_state system_state;
>> unsigned long spl; /* in mW */
>> unsigned long sppt; /* in mW */
>> diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
>> index 1608996654e8..eefffff83a4c 100644
>> --- a/drivers/platform/x86/amd/pmf/tee-if.c
>> +++ b/drivers/platform/x86/amd/pmf/tee-if.c
>> @@ -79,10 +79,10 @@ static int amd_pmf_update_uevents(struct amd_pmf_dev *dev, u16 event)
>> return 0;
>> }
>>
>> -static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_result *out)
>> +static int amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_result *out)
>> {
>> u32 val, event = 0;
>> - int idx;
>> + int idx, ret;
>>
>> for (idx = 0; idx < out->actions_count; idx++) {
>> val = out->actions_list[idx].value;
>> @@ -160,8 +160,23 @@ static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_
>> dev->prev_data->system_state = 0;
>> }
>> break;
>> +
>> + case PMF_POLICY_DISPLAY_BRIGHTNESS:
>> + ret = amd_pmf_get_gfx_data(&dev->gfx_data);
>> + if (ret)
>> + return ret;
>> +
>> + dev->prev_data->display_brightness = dev->gfx_data.brightness;
>> + if (dev->prev_data->display_brightness != val) {
>> + dev->gfx_data.brightness = val;
>> + amd_pmf_set_gfx_data(&dev->gfx_data);
>> + dev_dbg(dev->dev, "update DISPLAY_BRIGHTNESS : %d\n", val);
>> + }
>> + break;
>> }
>> }
>> +
>> + return 0;
>> }
>>
>> static int amd_pmf_invoke_cmd_enact(struct amd_pmf_dev *dev)
>> diff --git a/include/linux/amd-pmf-io.h b/include/linux/amd-pmf-io.h
>> index a2d4af231362..ecae387ddaa6 100644
>> --- a/include/linux/amd-pmf-io.h
>> +++ b/include/linux/amd-pmf-io.h
>> @@ -25,4 +25,5 @@ struct amd_gpu_pmf_data {
>> };
>>
>> int amd_pmf_get_gfx_data(struct amd_gpu_pmf_data *pmf);
>> +int amd_pmf_set_gfx_data(struct amd_gpu_pmf_data *pmf);
>> #endif
>
^ permalink raw reply
* Re: [PATCH 13/15] platform/x86/amd/pmf: Add PMF-AMDGPU set interface
From: Shyam Sundar S K @ 2023-09-26 11:17 UTC (permalink / raw)
To: Mario Limonciello, Deucher, Alexander, hdegoede@redhat.com,
markgross@kernel.org, Natikar, Basavaraj, jikos@kernel.org,
benjamin.tissoires@redhat.com, Koenig, Christian, Pan, Xinhui,
airlied@gmail.com, daniel@ffwll.ch
Cc: amd-gfx@lists.freedesktop.org,
platform-driver-x86@vger.kernel.org,
dri-devel@lists.freedesktop.org, Patil Rajesh,
linux-input@vger.kernel.org
In-Reply-To: <0ad32609-d47b-4114-8c51-09e96d7737f2@amd.com>
On 9/25/2023 10:00 PM, Mario Limonciello wrote:
> On 9/25/2023 11:27, Deucher, Alexander wrote:
>> [Public]
>>
>>> -----Original Message-----
>>> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of
>>> Shyam Sundar S K
>>> Sent: Friday, September 22, 2023 1:51 PM
>>> To: hdegoede@redhat.com; markgross@kernel.org; Natikar, Basavaraj
>>> <Basavaraj.Natikar@amd.com>; jikos@kernel.org;
>>> benjamin.tissoires@redhat.com; Deucher, Alexander
>>> <Alexander.Deucher@amd.com>; Koenig, Christian
>>> <Christian.Koenig@amd.com>; Pan, Xinhui <Xinhui.Pan@amd.com>;
>>> airlied@gmail.com; daniel@ffwll.ch
>>> Cc: S-k, Shyam-sundar <Shyam-sundar.S-k@amd.com>; amd-
>>> gfx@lists.freedesktop.org; platform-driver-x86@vger.kernel.org; dri-
>>> devel@lists.freedesktop.org; Patil Rajesh <Patil.Reddy@amd.com>;
>>> linux-
>>> input@vger.kernel.org; Limonciello, Mario <Mario.Limonciello@amd.com>
>>> Subject: [PATCH 13/15] platform/x86/amd/pmf: Add PMF-AMDGPU set
>>> interface
>>>
>>> For the Smart PC Solution to fully work, it has to enact to the
>>> actions coming
>>> from TA. Add the initial code path for set interface to AMDGPU.
>>
>> This seems to be limited to backlight at this point. What does
>> setting or not setting the backlight level mean for the system when
>> this framework is in place? What if a user manually changes the
>> backlight level? Additional comments below.
>>
>
> It's also for the display count.
display count is on the PMF-GPU GET interface. On the SET interface
its only backlight for today.
Thanks,
Shyam
>
>>>
>>> Co-developed-by: Mario Limonciello <mario.limonciello@amd.com>
>>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>>> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c | 21
>>> +++++++++++++++++++++
>>> drivers/platform/x86/amd/pmf/pmf.h | 2 ++
>>> drivers/platform/x86/amd/pmf/tee-if.c | 19 +++++++++++++++++--
>>> include/linux/amd-pmf-io.h | 1 +
>>> 4 files changed, 41 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
>>> index 232d11833ddc..5c567bff0548 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
>>> @@ -68,3 +68,24 @@ int amd_pmf_get_gfx_data(struct amd_gpu_pmf_data
>>> *pmf)
>>> return 0;
>>> }
>>> EXPORT_SYMBOL_GPL(amd_pmf_get_gfx_data);
>>> +
>>> +int amd_pmf_set_gfx_data(struct amd_gpu_pmf_data *pmf) {
>>> + struct drm_device *drm_dev = pci_get_drvdata(pmf->gpu_dev);
>>> + struct amdgpu_device *adev = drm_to_adev(drm_dev);
>>> + struct backlight_device *bd;
>>> +
>>> + if (!(adev->flags & AMD_IS_APU)) {
>>> + DRM_ERROR("PMF-AMDGPU interface not supported\n");
>>> + return -ENODEV;
>>> + }
>>> +
>>> + bd = backlight_device_get_by_type(BACKLIGHT_RAW);
>>> + if (!bd)
>>> + return -ENODEV;
>>> +
>>> + backlight_device_set_brightness(bd, pmf->brightness);
>>> +
>>> + return 0;
>>> +}
>>> +EXPORT_SYMBOL_GPL(amd_pmf_set_gfx_data);
>>> diff --git a/drivers/platform/x86/amd/pmf/pmf.h
>>> b/drivers/platform/x86/amd/pmf/pmf.h
>>> index 9032df4ba48a..ce89cc0daa5a 100644
>>> --- a/drivers/platform/x86/amd/pmf/pmf.h
>>> +++ b/drivers/platform/x86/amd/pmf/pmf.h
>>> @@ -73,6 +73,7 @@
>>> #define PMF_POLICY_STT_SKINTEMP_APU 7
>>> #define PMF_POLICY_STT_SKINTEMP_HS2 8
>>> #define
>>> PMF_POLICY_SYSTEM_STATE 9
>>> +#define
>>> PMF_POLICY_DISPLAY_BRIGHTNESS 12
>>> #define
>>> PMF_POLICY_P3T 38
>>>
>>> /* TA macros */
>>> @@ -480,6 +481,7 @@ enum ta_pmf_error_type { };
>>>
>>> struct pmf_action_table {
>>> + unsigned long display_brightness;
>>> enum system_state system_state;
>>> unsigned long spl; /* in mW */
>>> unsigned long sppt; /* in mW */
>>> diff --git a/drivers/platform/x86/amd/pmf/tee-if.c
>>> b/drivers/platform/x86/amd/pmf/tee-if.c
>>> index 1608996654e8..eefffff83a4c 100644
>>> --- a/drivers/platform/x86/amd/pmf/tee-if.c
>>> +++ b/drivers/platform/x86/amd/pmf/tee-if.c
>>> @@ -79,10 +79,10 @@ static int amd_pmf_update_uevents(struct
>>> amd_pmf_dev *dev, u16 event)
>>> return 0;
>>> }
>>>
>>> -static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct
>>> ta_pmf_enact_result *out)
>>> +static int amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct
>>> +ta_pmf_enact_result *out)
>>> {
>>> u32 val, event = 0;
>>> - int idx;
>>> + int idx, ret;
>>>
>>> for (idx = 0; idx < out->actions_count; idx++) {
>>> val = out->actions_list[idx].value;
>>> @@ -160,8 +160,23 @@ static void amd_pmf_apply_policies(struct
>>> amd_pmf_dev *dev, struct ta_pmf_enact_
>>> dev->prev_data->system_state = 0;
>>> }
>>> break;
>>> +
>>> + case PMF_POLICY_DISPLAY_BRIGHTNESS:
>>> + ret = amd_pmf_get_gfx_data(&dev->gfx_data);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + dev->prev_data->display_brightness = dev-
>>>> gfx_data.brightness;
>>
>> Are we using standardized units for the brightness? On the GPU
>> side, we align with the standard blacklight interface, but
>> internally, the driver has to convert units depending on the type of
>> backlight controller used on the platform. Presumably PMF does
>> something similar?
>>
>> Alex
>>
>>> + if (dev->prev_data->display_brightness != val) {
>>> + dev->gfx_data.brightness = val;
>>> + amd_pmf_set_gfx_data(&dev->gfx_data);
>>> + dev_dbg(dev->dev, "update
>>> DISPLAY_BRIGHTNESS : %d\n", val);
>>> + }
>>> + break;
>>> }
>>> }
>>> +
>>> + return 0;
>>> }
>>>
>>> static int amd_pmf_invoke_cmd_enact(struct amd_pmf_dev *dev) diff
>>> --git
>>> a/include/linux/amd-pmf-io.h b/include/linux/amd-pmf-io.h index
>>> a2d4af231362..ecae387ddaa6 100644
>>> --- a/include/linux/amd-pmf-io.h
>>> +++ b/include/linux/amd-pmf-io.h
>>> @@ -25,4 +25,5 @@ struct amd_gpu_pmf_data { };
>>>
>>> int amd_pmf_get_gfx_data(struct amd_gpu_pmf_data *pmf);
>>> +int amd_pmf_set_gfx_data(struct amd_gpu_pmf_data *pmf);
>>> #endif
>>> --
>>> 2.25.1
>>
>
^ permalink raw reply
* Re: [PATCH 13/15] platform/x86/amd/pmf: Add PMF-AMDGPU set interface
From: Shyam Sundar S K @ 2023-09-26 11:15 UTC (permalink / raw)
To: Deucher, Alexander, hdegoede@redhat.com, markgross@kernel.org,
Natikar, Basavaraj, jikos@kernel.org,
benjamin.tissoires@redhat.com, Koenig, Christian, Pan, Xinhui,
airlied@gmail.com, daniel@ffwll.ch
Cc: amd-gfx@lists.freedesktop.org,
platform-driver-x86@vger.kernel.org,
dri-devel@lists.freedesktop.org, Patil Rajesh,
linux-input@vger.kernel.org, Limonciello, Mario
In-Reply-To: <BL1PR12MB51442D18C314B1E34B77DE2BF7FCA@BL1PR12MB5144.namprd12.prod.outlook.com>
On 9/25/2023 9:57 PM, Deucher, Alexander wrote:
> [Public]
>
>> -----Original Message-----
>> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of
>> Shyam Sundar S K
>> Sent: Friday, September 22, 2023 1:51 PM
>> To: hdegoede@redhat.com; markgross@kernel.org; Natikar, Basavaraj
>> <Basavaraj.Natikar@amd.com>; jikos@kernel.org;
>> benjamin.tissoires@redhat.com; Deucher, Alexander
>> <Alexander.Deucher@amd.com>; Koenig, Christian
>> <Christian.Koenig@amd.com>; Pan, Xinhui <Xinhui.Pan@amd.com>;
>> airlied@gmail.com; daniel@ffwll.ch
>> Cc: S-k, Shyam-sundar <Shyam-sundar.S-k@amd.com>; amd-
>> gfx@lists.freedesktop.org; platform-driver-x86@vger.kernel.org; dri-
>> devel@lists.freedesktop.org; Patil Rajesh <Patil.Reddy@amd.com>; linux-
>> input@vger.kernel.org; Limonciello, Mario <Mario.Limonciello@amd.com>
>> Subject: [PATCH 13/15] platform/x86/amd/pmf: Add PMF-AMDGPU set
>> interface
>>
>> For the Smart PC Solution to fully work, it has to enact to the actions coming
>> from TA. Add the initial code path for set interface to AMDGPU.
>
> This seems to be limited to backlight at this point. What does setting or not setting the backlight level mean for the system when this framework is in place? What if a user manually changes the backlight level? Additional comments below.
The unit here is nits that varies from 0 to 255. User can manually
update the backlight but if there is an action from the TA to update
the backlight, PMF driver would send a request to GPU driver to update
the backlight to the updated value (in nits)
At this point, yes. PMF is using to PMF-GPU interface to set
backlight, but there are additional things to be added in future. This
patch builds the initial plumbing for that.
>
>>
>> Co-developed-by: Mario Limonciello <mario.limonciello@amd.com>
>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c | 21
>> +++++++++++++++++++++
>> drivers/platform/x86/amd/pmf/pmf.h | 2 ++
>> drivers/platform/x86/amd/pmf/tee-if.c | 19 +++++++++++++++++--
>> include/linux/amd-pmf-io.h | 1 +
>> 4 files changed, 41 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
>> index 232d11833ddc..5c567bff0548 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
>> @@ -68,3 +68,24 @@ int amd_pmf_get_gfx_data(struct amd_gpu_pmf_data
>> *pmf)
>> return 0;
>> }
>> EXPORT_SYMBOL_GPL(amd_pmf_get_gfx_data);
>> +
>> +int amd_pmf_set_gfx_data(struct amd_gpu_pmf_data *pmf) {
>> + struct drm_device *drm_dev = pci_get_drvdata(pmf->gpu_dev);
>> + struct amdgpu_device *adev = drm_to_adev(drm_dev);
>> + struct backlight_device *bd;
>> +
>> + if (!(adev->flags & AMD_IS_APU)) {
>> + DRM_ERROR("PMF-AMDGPU interface not supported\n");
>> + return -ENODEV;
>> + }
>> +
>> + bd = backlight_device_get_by_type(BACKLIGHT_RAW);
>> + if (!bd)
>> + return -ENODEV;
>> +
>> + backlight_device_set_brightness(bd, pmf->brightness);
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(amd_pmf_set_gfx_data);
>> diff --git a/drivers/platform/x86/amd/pmf/pmf.h
>> b/drivers/platform/x86/amd/pmf/pmf.h
>> index 9032df4ba48a..ce89cc0daa5a 100644
>> --- a/drivers/platform/x86/amd/pmf/pmf.h
>> +++ b/drivers/platform/x86/amd/pmf/pmf.h
>> @@ -73,6 +73,7 @@
>> #define PMF_POLICY_STT_SKINTEMP_APU 7
>> #define PMF_POLICY_STT_SKINTEMP_HS2 8
>> #define PMF_POLICY_SYSTEM_STATE 9
>> +#define PMF_POLICY_DISPLAY_BRIGHTNESS 12
>> #define PMF_POLICY_P3T 38
>>
>> /* TA macros */
>> @@ -480,6 +481,7 @@ enum ta_pmf_error_type { };
>>
>> struct pmf_action_table {
>> + unsigned long display_brightness;
>> enum system_state system_state;
>> unsigned long spl; /* in mW */
>> unsigned long sppt; /* in mW */
>> diff --git a/drivers/platform/x86/amd/pmf/tee-if.c
>> b/drivers/platform/x86/amd/pmf/tee-if.c
>> index 1608996654e8..eefffff83a4c 100644
>> --- a/drivers/platform/x86/amd/pmf/tee-if.c
>> +++ b/drivers/platform/x86/amd/pmf/tee-if.c
>> @@ -79,10 +79,10 @@ static int amd_pmf_update_uevents(struct
>> amd_pmf_dev *dev, u16 event)
>> return 0;
>> }
>>
>> -static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct
>> ta_pmf_enact_result *out)
>> +static int amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct
>> +ta_pmf_enact_result *out)
>> {
>> u32 val, event = 0;
>> - int idx;
>> + int idx, ret;
>>
>> for (idx = 0; idx < out->actions_count; idx++) {
>> val = out->actions_list[idx].value;
>> @@ -160,8 +160,23 @@ static void amd_pmf_apply_policies(struct
>> amd_pmf_dev *dev, struct ta_pmf_enact_
>> dev->prev_data->system_state = 0;
>> }
>> break;
>> +
>> + case PMF_POLICY_DISPLAY_BRIGHTNESS:
>> + ret = amd_pmf_get_gfx_data(&dev->gfx_data);
>> + if (ret)
>> + return ret;
>> +
>> + dev->prev_data->display_brightness = dev-
>>> gfx_data.brightness;
>
> Are we using standardized units for the brightness? On the GPU side, we align with the standard blacklight interface, but internally, the driver has to convert units depending on the type of backlight controller used on the platform. Presumably PMF does something similar?
Yes its the standard nits. There is no conversion needed.
>
> Alex
>
>> + if (dev->prev_data->display_brightness != val) {
>> + dev->gfx_data.brightness = val;
>> + amd_pmf_set_gfx_data(&dev->gfx_data);
>> + dev_dbg(dev->dev, "update
>> DISPLAY_BRIGHTNESS : %d\n", val);
>> + }
>> + break;
>> }
>> }
>> +
>> + return 0;
>> }
>>
>> static int amd_pmf_invoke_cmd_enact(struct amd_pmf_dev *dev) diff --git
>> a/include/linux/amd-pmf-io.h b/include/linux/amd-pmf-io.h index
>> a2d4af231362..ecae387ddaa6 100644
>> --- a/include/linux/amd-pmf-io.h
>> +++ b/include/linux/amd-pmf-io.h
>> @@ -25,4 +25,5 @@ struct amd_gpu_pmf_data { };
>>
>> int amd_pmf_get_gfx_data(struct amd_gpu_pmf_data *pmf);
>> +int amd_pmf_set_gfx_data(struct amd_gpu_pmf_data *pmf);
>> #endif
>> --
>> 2.25.1
>
^ permalink raw reply
* Re: [PATCH] Input: i8042 - add quirk for Lenovo ThinkPad T14 Gen 1
From: Hans de Goede @ 2023-09-26 10:37 UTC (permalink / raw)
To: Jonathan Denose, linux-input
Cc: Jonathan Denose, Dmitry Torokhov, Huacai Chen,
Mattijs Korpershoek, Takashi Iwai, Werner Sembach, linux-kernel
In-Reply-To: <20230925163313.1.I55bfb5880d6755094a995d3ae44c13810ae98be4@changeid>
Hi,
On 9/25/23 23:33, Jonathan Denose wrote:
> The ThinkPad T14 Gen 1 touchpad works fine except that clicking
> and dragging by tapping the touchpad or depressing the touchpad
> do not work. Disabling PNP for controller setting discovery enables
> click and drag without negatively impacting other touchpad features.
>
> Signed-off-by: Jonathan Denose <jdenose@google.com>
Thanks, patch looks good to me:
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Regards,
Hans
> ---
>
> drivers/input/serio/i8042-acpipnpio.h | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/input/serio/i8042-acpipnpio.h b/drivers/input/serio/i8042-acpipnpio.h
> index 1724d6cb8649d..c487047d878cd 100644
> --- a/drivers/input/serio/i8042-acpipnpio.h
> +++ b/drivers/input/serio/i8042-acpipnpio.h
> @@ -830,6 +830,14 @@ static const struct dmi_system_id i8042_dmi_quirk_table[] __initconst = {
> },
> .driver_data = (void *)(SERIO_QUIRK_RESET_ALWAYS)
> },
> + {
> + /* Lenovo ThinkPad T14 Gen 1*/
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
> + DMI_MATCH(DMI_PRODUCT_NAME, "20S0002UUS")
> + },
> + .driver_data = (void *)(SERIO_QUIRK_NOPNP)
> + },
> {
> /* Lenovo ThinkPad Twist S230u */
> .matches = {
^ permalink raw reply
* Re: [PATCH 13/15] platform/x86/amd/pmf: Add PMF-AMDGPU set interface
From: Hans de Goede @ 2023-09-26 10:35 UTC (permalink / raw)
To: Shyam Sundar S K, markgross, basavaraj.natikar, jikos,
benjamin.tissoires, alexander.deucher, christian.koenig,
Xinhui.Pan, airlied, daniel
Cc: Patil.Reddy, mario.limonciello, platform-driver-x86, linux-input,
amd-gfx, dri-devel
In-Reply-To: <20230922175056.244940-14-Shyam-sundar.S-k@amd.com>
Hi,
On 9/22/23 19:50, Shyam Sundar S K wrote:
> For the Smart PC Solution to fully work, it has to enact to the actions
> coming from TA. Add the initial code path for set interface to AMDGPU.
>
> Co-developed-by: Mario Limonciello <mario.limonciello@amd.com>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c | 21 +++++++++++++++++++++
> drivers/platform/x86/amd/pmf/pmf.h | 2 ++
> drivers/platform/x86/amd/pmf/tee-if.c | 19 +++++++++++++++++--
> include/linux/amd-pmf-io.h | 1 +
> 4 files changed, 41 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
> index 232d11833ddc..5c567bff0548 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
> @@ -68,3 +68,24 @@ int amd_pmf_get_gfx_data(struct amd_gpu_pmf_data *pmf)
> return 0;
> }
> EXPORT_SYMBOL_GPL(amd_pmf_get_gfx_data);
> +
> +int amd_pmf_set_gfx_data(struct amd_gpu_pmf_data *pmf)
> +{
> + struct drm_device *drm_dev = pci_get_drvdata(pmf->gpu_dev);
> + struct amdgpu_device *adev = drm_to_adev(drm_dev);
> + struct backlight_device *bd;
> +
> + if (!(adev->flags & AMD_IS_APU)) {
> + DRM_ERROR("PMF-AMDGPU interface not supported\n");
> + return -ENODEV;
> + }
> +
> + bd = backlight_device_get_by_type(BACKLIGHT_RAW);
> + if (!bd)
> + return -ENODEV;
This assumes that the backlight is always controller by the amdgpu's
native backlight driver, but it might e.g. also be handled by
eacpi-video or by nvidia_wmi_ec_backlight (when using an AMD APU +
nvidia dgpu).
For now what should be done here is to call acpi_video_get_backlight_type()
and then translate the return value from this into a backlight-type:
acpi_backlight_video -> BACKLIGHT_FIRMWARE
acpi_backlight_vendor, -> BACKLIGHT_PLATFORM
acpi_backlight_native, -> BACKLIGHT_RAW
acpi_backlight_nvidia_wmi_ec, -> BACKLIGHT_FIRMWARE
acpi_backlight_apple_gmux, -> BACKLIGHT_PLATFORM
Also I'm worried about probe order here, this code currently assumes
that the GPU or other backlight driver has loaded before this runs,
which is not necessarily the case.
I think that if the backlight_device_get_by_type() fails this
should be retried say every 10 seconds from some delayed workqueue
for at least a couple of minutes after boot.
Regards,
Hans
> +
> + backlight_device_set_brightness(bd, pmf->brightness);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(amd_pmf_set_gfx_data);
> diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
> index 9032df4ba48a..ce89cc0daa5a 100644
> --- a/drivers/platform/x86/amd/pmf/pmf.h
> +++ b/drivers/platform/x86/amd/pmf/pmf.h
> @@ -73,6 +73,7 @@
> #define PMF_POLICY_STT_SKINTEMP_APU 7
> #define PMF_POLICY_STT_SKINTEMP_HS2 8
> #define PMF_POLICY_SYSTEM_STATE 9
> +#define PMF_POLICY_DISPLAY_BRIGHTNESS 12
> #define PMF_POLICY_P3T 38
>
> /* TA macros */
> @@ -480,6 +481,7 @@ enum ta_pmf_error_type {
> };
>
> struct pmf_action_table {
> + unsigned long display_brightness;
> enum system_state system_state;
> unsigned long spl; /* in mW */
> unsigned long sppt; /* in mW */
> diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
> index 1608996654e8..eefffff83a4c 100644
> --- a/drivers/platform/x86/amd/pmf/tee-if.c
> +++ b/drivers/platform/x86/amd/pmf/tee-if.c
> @@ -79,10 +79,10 @@ static int amd_pmf_update_uevents(struct amd_pmf_dev *dev, u16 event)
> return 0;
> }
>
> -static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_result *out)
> +static int amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_result *out)
> {
> u32 val, event = 0;
> - int idx;
> + int idx, ret;
>
> for (idx = 0; idx < out->actions_count; idx++) {
> val = out->actions_list[idx].value;
> @@ -160,8 +160,23 @@ static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_
> dev->prev_data->system_state = 0;
> }
> break;
> +
> + case PMF_POLICY_DISPLAY_BRIGHTNESS:
> + ret = amd_pmf_get_gfx_data(&dev->gfx_data);
> + if (ret)
> + return ret;
> +
> + dev->prev_data->display_brightness = dev->gfx_data.brightness;
> + if (dev->prev_data->display_brightness != val) {
> + dev->gfx_data.brightness = val;
> + amd_pmf_set_gfx_data(&dev->gfx_data);
> + dev_dbg(dev->dev, "update DISPLAY_BRIGHTNESS : %d\n", val);
> + }
> + break;
> }
> }
> +
> + return 0;
> }
>
> static int amd_pmf_invoke_cmd_enact(struct amd_pmf_dev *dev)
> diff --git a/include/linux/amd-pmf-io.h b/include/linux/amd-pmf-io.h
> index a2d4af231362..ecae387ddaa6 100644
> --- a/include/linux/amd-pmf-io.h
> +++ b/include/linux/amd-pmf-io.h
> @@ -25,4 +25,5 @@ struct amd_gpu_pmf_data {
> };
>
> int amd_pmf_get_gfx_data(struct amd_gpu_pmf_data *pmf);
> +int amd_pmf_set_gfx_data(struct amd_gpu_pmf_data *pmf);
> #endif
^ permalink raw reply
* Re: [PATCH V2 1/2] dt-bindings: input: Introduce Himax HID-over-SPI device
From: yang tylor @ 2023-09-26 9:52 UTC (permalink / raw)
To: Conor Dooley
Cc: Conor Dooley, dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt,
conor+dt, jikos, benjamin.tissoires, linux-input, devicetree,
linux-kernel, poyuan_chang, hbarnor, jingyliang@chromium.org,
wuxy23, luolm1, hung poyu
In-Reply-To: <20230926-action-sludge-ec8e51fdd6d4@spud>
On Tue, Sep 26, 2023 at 5:02 PM Conor Dooley <conor@kernel.org> wrote:
>
> On Mon, Sep 25, 2023 at 06:16:29PM +0800, yang tylor wrote:
> > On Mon, Sep 25, 2023 at 4:41 PM Conor Dooley <conor.dooley@microchip.com> wrote:
> > >
> > > On Mon, Sep 25, 2023 at 09:44:21AM +0800, yang tylor wrote:
> > > > On Fri, Sep 22, 2023 at 11:31 PM Conor Dooley <conor@kernel.org> wrote:
> > > > >
> > > > > On Fri, Sep 22, 2023 at 05:43:54PM +0800, yang tylor wrote:
> > > > > > On Fri, Sep 22, 2023 at 5:22 PM Conor Dooley <conor@kernel.org> wrote:
> > > > > > >
> > > > > > > On Fri, Sep 22, 2023 at 03:56:25PM +0800, yang tylor wrote:
> > > > > > > > On Tue, Sep 19, 2023 at 7:09 PM Conor Dooley <conor@kernel.org> wrote:
> > > > > > > > > On Tue, Sep 19, 2023 at 05:31:29PM +0800, yang tylor wrote:
> > > > > > >
> > > > > > > > > > The behavior of "himax,boot_time_fw_upgrade" seems not stable and
> > > > > > > > > > should be removed. "himax,fw_in_flash", I use the kernel config for
> > > > > > > > > > user to select.
> > > > > > > > >
> > > > > > > > > That seems like a bad idea, we want to be able to build one kernel that
> > > > > > > > > works for all hardware at the same time.
> > > > > > > > >
> > > > > > > > I see, so I should take that back?
> > > > > > > > I'll explain more about it.
> > > > > > >
> > > > > > > Are there particular ICs where the firmware would always be in flash and
> > > > > > > others where it would never be? Or is this a choice made by the board or
> > > > > > > system designer?
> > > > > > >
> > > > > > Most cases it's about the system designer's decision. But some ICs may be forced
> > > > > > to use flash because of its architecture(multiple IC inside, need to
> > > > > > load firmware to
> > > > > > multiple IC's sram by master IC). But if there is no limitation on
> > > > > > this part, most system
> > > > > > designers will prefer flashless.
> > > > >
> > > > > Forgive me if I am not understanding correctly, there are some ICs that
> > > > > will need to load the firmware from flash and there are some where it
> > > > > will be a decision made by the designer of the board. Is the flash part
> > > > > of the IC or is it an external flash chip?
> > > > >
> > > >
> > > > Both are possible, it depends on the IC type. For TDDI, the IC is long
> > > > and thin, placed on panel PCB, flash will be located at the external
> > > > flash chip. For the OLED TP, IC is usually placed at FPC and its flash
> > > > is embedded, thus the IC size is large compared to TDDI. But from the
> > > > driver's perspective either external flash or embedded flash, the IC
> > > > itself will load firmware from flash automatically when reset pin is
> > > > released. Only if firmware is loading from the host storage system,
> > > > the driver needs to operate the IC in detail.
> > >
> > >
> > > Since there are ICs that can use the external flash or have it loaded
> > > from the host, it sounds like you do need a property to differentiate
> > > between those cases.
> > Yep.
> >
> > > Is it sufficient to just set the firmware-name property for these cases?
> > > If the property exists, then you know you need to load firmware & what
> > > its name is. If it doesn't, then the firmware either isn't needed or
> > > will be automatically loaded from the external flash.
>
> > We have a default prefix firmware name(like himax_xxxx.bin) in the driver code.
>
> How do you intend generating the name of the firmware file? I assume the
> same firmware doesn't work on every IC, so you'll need to pick a
> different one depending on the compatible?
>
If considering a firmware library line-up for all the incoming panels
of this driver.
We would use PID as part of the file name. Because all the support panels would
have a unique PID associated. Which will make the firmware name like
himax_xxx_{$PID}.bin. The problem is, we need to know PID before firmware load
at no flash condition. Thus PID information is required in dts when
no-flash-flag
is specified.
> > So we'll look for it when no-flash-flag is specified. In our experience,
> > forcing a prefix firmware name helps the user to aware what firmware
> > they are dealing with.
If a more simple solution for no-flash condition is needed, as you mentioned,
specifying a firmware name in dts would be the best. Otherwise, a
no-flash-flag and
PID information needs to be added in dts.
Thanks,
Tylor
^ permalink raw reply
* Re: [PATCH V2 1/2] dt-bindings: input: Introduce Himax HID-over-SPI device
From: Conor Dooley @ 2023-09-26 9:02 UTC (permalink / raw)
To: yang tylor
Cc: Conor Dooley, dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt,
conor+dt, jikos, benjamin.tissoires, linux-input, devicetree,
linux-kernel, poyuan_chang, hbarnor, jingyliang@chromium.org,
wuxy23, luolm1, hung poyu
In-Reply-To: <CAGD2q_a1nLtFj7H42f+u+J5Bih59MGS0aJLHCFJy5gM2ydys4w@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3628 bytes --]
On Mon, Sep 25, 2023 at 06:16:29PM +0800, yang tylor wrote:
> On Mon, Sep 25, 2023 at 4:41 PM Conor Dooley <conor.dooley@microchip.com> wrote:
> >
> > On Mon, Sep 25, 2023 at 09:44:21AM +0800, yang tylor wrote:
> > > On Fri, Sep 22, 2023 at 11:31 PM Conor Dooley <conor@kernel.org> wrote:
> > > >
> > > > On Fri, Sep 22, 2023 at 05:43:54PM +0800, yang tylor wrote:
> > > > > On Fri, Sep 22, 2023 at 5:22 PM Conor Dooley <conor@kernel.org> wrote:
> > > > > >
> > > > > > On Fri, Sep 22, 2023 at 03:56:25PM +0800, yang tylor wrote:
> > > > > > > On Tue, Sep 19, 2023 at 7:09 PM Conor Dooley <conor@kernel.org> wrote:
> > > > > > > > On Tue, Sep 19, 2023 at 05:31:29PM +0800, yang tylor wrote:
> > > > > >
> > > > > > > > > The behavior of "himax,boot_time_fw_upgrade" seems not stable and
> > > > > > > > > should be removed. "himax,fw_in_flash", I use the kernel config for
> > > > > > > > > user to select.
> > > > > > > >
> > > > > > > > That seems like a bad idea, we want to be able to build one kernel that
> > > > > > > > works for all hardware at the same time.
> > > > > > > >
> > > > > > > I see, so I should take that back?
> > > > > > > I'll explain more about it.
> > > > > >
> > > > > > Are there particular ICs where the firmware would always be in flash and
> > > > > > others where it would never be? Or is this a choice made by the board or
> > > > > > system designer?
> > > > > >
> > > > > Most cases it's about the system designer's decision. But some ICs may be forced
> > > > > to use flash because of its architecture(multiple IC inside, need to
> > > > > load firmware to
> > > > > multiple IC's sram by master IC). But if there is no limitation on
> > > > > this part, most system
> > > > > designers will prefer flashless.
> > > >
> > > > Forgive me if I am not understanding correctly, there are some ICs that
> > > > will need to load the firmware from flash and there are some where it
> > > > will be a decision made by the designer of the board. Is the flash part
> > > > of the IC or is it an external flash chip?
> > > >
> > >
> > > Both are possible, it depends on the IC type. For TDDI, the IC is long
> > > and thin, placed on panel PCB, flash will be located at the external
> > > flash chip. For the OLED TP, IC is usually placed at FPC and its flash
> > > is embedded, thus the IC size is large compared to TDDI. But from the
> > > driver's perspective either external flash or embedded flash, the IC
> > > itself will load firmware from flash automatically when reset pin is
> > > released. Only if firmware is loading from the host storage system,
> > > the driver needs to operate the IC in detail.
> >
> >
> > Since there are ICs that can use the external flash or have it loaded
> > from the host, it sounds like you do need a property to differentiate
> > between those cases.
> Yep.
>
> > Is it sufficient to just set the firmware-name property for these cases?
> > If the property exists, then you know you need to load firmware & what
> > its name is. If it doesn't, then the firmware either isn't needed or
> > will be automatically loaded from the external flash.
> We have a default prefix firmware name(like himax_xxxx.bin) in the driver code.
How do you intend generating the name of the firmware file? I assume the
same firmware doesn't work on every IC, so you'll need to pick a
different one depending on the compatible?
> So we'll look for it when no-flash-flag is specified. In our experience,
> forcing a prefix firmware name helps the user to aware what firmware
> they are dealing with.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v2 2/2] Input: ilitek_ts_i2c - add report id message validation
From: Francesco Dolcini @ 2023-09-26 8:14 UTC (permalink / raw)
To: Emanuele Ghidoli
Cc: Dmitry Torokhov, Emanuele Ghidoli, linux-kernel, linux-input,
Joe Hung
In-Reply-To: <20230920074650.922292-3-ghidoliemanuele@gmail.com>
On Wed, Sep 20, 2023 at 09:46:50AM +0200, Emanuele Ghidoli wrote:
> From: Emanuele Ghidoli <emanuele.ghidoli@toradex.com>
>
> Ilitek touch IC driver answer to plain i2c read request, after it has
> generated an interrupt request, with a report id message starting
> with an identifier and a series of points.
> If a request is sent unsolicited by an interrupt request the answer
> do not contain this identifier.
> Add a test to discard these messages, making the driver robust to
> spurious interrupt requests.
>
> Fixes: 42370681bd46 ("Input: Add support for ILITEK Lego Series")
> Signed-off-by: Emanuele Ghidoli <emanuele.ghidoli@toradex.com>
Reviewed-by: Francesco Dolcini <francesco.dolcini@toradex.com>
Francesco
^ permalink raw reply
* Re: [PATCH v2 1/2] Input: ilitek_ts_i2c - avoid wrong input subsystem sync
From: Francesco Dolcini @ 2023-09-26 8:15 UTC (permalink / raw)
To: Emanuele Ghidoli
Cc: Dmitry Torokhov, Emanuele Ghidoli, linux-kernel, linux-input,
Joe Hung
In-Reply-To: <20230920074650.922292-2-ghidoliemanuele@gmail.com>
On Wed, Sep 20, 2023 at 09:46:49AM +0200, Emanuele Ghidoli wrote:
> From: Emanuele Ghidoli <emanuele.ghidoli@toradex.com>
>
> For different reasons i2c transaction may fail or
> report id message content may be wrong.
> Avoid sync the input subsystem if message cannot be parsed.
> An input subsystem sync without points is interpreted as
> "nothing is touching the screen" while normally this is not the case.
>
> Fixes: 42370681bd46 ("Input: Add support for ILITEK Lego Series")
> Signed-off-by: Emanuele Ghidoli <emanuele.ghidoli@toradex.com>
Reviewed-by: Francesco Dolcini <francesco.dolcini@toradex.com>
Francesco
^ permalink raw reply
* Re: [PATCH v2 0/3] selftests/hid: fix building for older kernels
From: Justin Stitt @ 2023-09-26 7:26 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Shuah Khan
Cc: Eduard Zingerman, linux-input, linux-kselftest, linux-kernel, bpf,
Benjamin Tissoires
In-Reply-To: <20230908-kselftest-09-08-v2-0-0def978a4c1b@google.com>
Hey all,
Gentle ping on this patch. Looking to get this patch and [1] slated
for 6.7 wherein we can start getting cleaner kselftests builds.
I do not think I am able to successfully run the hid/bpf selftests due
to my kernel version being too low (and an inability to upgrade it as
I'm on a corp rolling release). I'd appreciate some insight on how to
get the tests running or if someone could actually build+run the tests
with this patch applied.
On Sat, Sep 9, 2023 at 7:22 AM Justin Stitt <justinstitt@google.com> wrote:
>
> Hi, I am sending this series on behalf of myself and Benjamin Tissoires. There
> existed an initial n=3 patch series which was later expanded to n=4 and
> is now back to n=3 with some fixes added in and rebased against
> mainline.
>
> This patch series aims to ensure that the hid/bpf selftests can be built
> without errors.
>
> Here's Benjamin's initial cover letter for context:
> | These fixes have been triggered by [0]:
> | basically, if you do not recompile the kernel first, and are
> | running on an old kernel, vmlinux.h doesn't have the required
> | symbols and the compilation fails.
> |
> | The tests will fail if you run them on that very same machine,
> | of course, but the binary should compile.
> |
> | And while I was sorting out why it was failing, I realized I
> | could do a couple of improvements on the Makefile.
> |
> | [0] https://lore.kernel.org/linux-input/56ba8125-2c6f-a9c9-d498-0ca1c153dcb2@redhat.com/T/#t
>
> Changes from v1 -> v2:
> - roll Justin's fix into patch 1/3
> - add __attribute__((preserve_access_index)) (thanks Eduard)
> - rebased onto mainline (2dde18cd1d8fac735875f2e4987f11817cc0bc2c)
> - Link to v1: https://lore.kernel.org/all/20230825-wip-selftests-v1-0-c862769020a8@kernel.org/
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/1698
> Link: https://github.com/ClangBuiltLinux/continuous-integration2/issues/61
> ---
> Benjamin Tissoires (3):
> selftests/hid: ensure we can compile the tests on kernels pre-6.3
> selftests/hid: do not manually call headers_install
> selftests/hid: force using our compiled libbpf headers
>
> tools/testing/selftests/hid/Makefile | 10 ++---
> tools/testing/selftests/hid/progs/hid.c | 3 --
> .../testing/selftests/hid/progs/hid_bpf_helpers.h | 49 ++++++++++++++++++++++
> 3 files changed, 53 insertions(+), 9 deletions(-)
> ---
> base-commit: 2dde18cd1d8fac735875f2e4987f11817cc0bc2c
> change-id: 20230908-kselftest-09-08-56d7f4a8d5c4
>
> Best regards,
> --
> Justin Stitt <justinstitt@google.com>
>
[1]: https://lore.kernel.org/all/20230912-kselftest-param_test-c-v1-1-80a6cffc7374@google.com/
Thanks
Justin
^ permalink raw reply
* [PATCH v3 2/2] Input: gpio-keys - Add system suspend support for dedicated wakeirqs
From: Tony Lindgren @ 2023-09-26 6:38 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-input,
devicetree, linux-kernel, linux-arm-kernel, Rob Herring,
Dhruva Gole
In-Reply-To: <20230926063809.25654-1-tony@atomide.com>
Some SoCs have a separate dedicated wake-up interrupt controller that can
be used to wake up the system from deeper idle states. We already support
configuring a separate interrupt for a gpio-keys button to be used with a
gpio line. However, we are lacking support system suspend for cases where
a separate interrupt needs to be used in deeper sleep modes.
Because of it's nature, gpio-keys does not know about the runtime PM state
of the button gpios, and may have several gpio buttons configured for each
gpio-keys device instance. Implementing runtime PM support for gpio-keys
does not help, and we cannot use drivers/base/power/wakeirq.c support. We
need to implement custom wakeirq support for gpio-keys.
For handling a dedicated wakeirq for system suspend, we enable and disable
it with gpio_keys_enable_wakeup() and gpio_keys_disable_wakeup() that we
already use based on device_may_wakeup().
Some systems may have a dedicated wakeirq that can also be used as the
main interrupt, this is already working for gpio-keys. Let's add some
wakeirq related comments while at it as the usage with a gpio line and
separate interrupt line may not be obvious.
Tested-by: Dhruva Gole <d-gole@ti.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
No changes from v1
---
drivers/input/keyboard/gpio_keys.c | 69 ++++++++++++++++++++++++++++--
include/linux/gpio_keys.h | 2 +
2 files changed, 67 insertions(+), 4 deletions(-)
diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -45,7 +45,9 @@ struct gpio_button_data {
unsigned int software_debounce; /* in msecs, for GPIO-driven buttons */
unsigned int irq;
+ unsigned int wakeirq;
unsigned int wakeup_trigger_type;
+
spinlock_t lock;
bool disabled;
bool key_pressed;
@@ -511,6 +513,7 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
struct gpio_button_data *bdata = &ddata->data[idx];
irq_handler_t isr;
unsigned long irqflags;
+ const char *wakedesc;
int irq;
int error;
@@ -575,6 +578,14 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
!gpiod_cansleep(bdata->gpiod);
}
+ /*
+ * If an interrupt was specified, use it instead of the gpio
+ * interrupt and use the gpio for reading the state. A separate
+ * interrupt may be used as the main button interrupt for
+ * runtime PM to detect events also in deeper idle states. If a
+ * dedicated wakeirq is used for system suspend only, see below
+ * for bdata->wakeirq setup.
+ */
if (button->irq) {
bdata->irq = button->irq;
} else {
@@ -672,6 +683,36 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
return error;
}
+ if (!button->wakeirq)
+ return 0;
+
+ /* Use :wakeup suffix like drivers/base/power/wakeirq.c does */
+ wakedesc = devm_kasprintf(dev, GFP_KERNEL, "%s:wakeup", desc);
+ if (!wakedesc)
+ return -ENOMEM;
+
+ bdata->wakeirq = button->wakeirq;
+ irqflags |= IRQF_NO_SUSPEND;
+
+ /*
+ * Wakeirq shares the handler with the main interrupt, it's only
+ * active during system suspend. See gpio_keys_button_enable_wakeup()
+ * and gpio_keys_button_disable_wakeup().
+ */
+ error = devm_request_any_context_irq(dev, bdata->wakeirq, isr,
+ irqflags, wakedesc, bdata);
+ if (error < 0) {
+ dev_err(dev, "Unable to claim wakeirq %d; error %d\n",
+ bdata->irq, error);
+ return error;
+ }
+
+ /*
+ * Disable wakeirq until suspend. IRQF_NO_AUTOEN won't work if
+ * IRQF_SHARED was set based on !button->can_disable.
+ */
+ disable_irq_nosync(bdata->wakeirq);
+
return 0;
}
@@ -728,7 +769,7 @@ gpio_keys_get_devtree_pdata(struct device *dev)
struct gpio_keys_platform_data *pdata;
struct gpio_keys_button *button;
struct fwnode_handle *child;
- int nbuttons;
+ int nbuttons, irq;
nbuttons = device_get_child_node_count(dev);
if (nbuttons == 0)
@@ -750,9 +791,19 @@ gpio_keys_get_devtree_pdata(struct device *dev)
device_property_read_string(dev, "label", &pdata->name);
device_for_each_child_node(dev, child) {
- if (is_of_node(child))
- button->irq =
- irq_of_parse_and_map(to_of_node(child), 0);
+ if (is_of_node(child)) {
+ irq = of_irq_get_byname(to_of_node(child), "irq");
+ if (irq > 0)
+ button->irq = irq;
+
+ irq = of_irq_get_byname(to_of_node(child), "wakeup");
+ if (irq > 0)
+ button->wakeirq = irq;
+
+ if (!button->irq && !button->wakeirq)
+ button->irq =
+ irq_of_parse_and_map(to_of_node(child), 0);
+ }
if (fwnode_property_read_u32(child, "linux,code",
&button->code)) {
@@ -921,6 +972,11 @@ gpio_keys_button_enable_wakeup(struct gpio_button_data *bdata)
}
}
+ if (bdata->wakeirq) {
+ enable_irq(bdata->wakeirq);
+ disable_irq_nosync(bdata->irq);
+ }
+
return 0;
}
@@ -929,6 +985,11 @@ gpio_keys_button_disable_wakeup(struct gpio_button_data *bdata)
{
int error;
+ if (bdata->wakeirq) {
+ enable_irq(bdata->irq);
+ disable_irq_nosync(bdata->wakeirq);
+ }
+
/*
* The trigger type is always both edges for gpio-based keys and we do
* not support changing wakeup trigger for interrupt-based keys.
diff --git a/include/linux/gpio_keys.h b/include/linux/gpio_keys.h
--- a/include/linux/gpio_keys.h
+++ b/include/linux/gpio_keys.h
@@ -21,6 +21,7 @@ struct device;
* disable button via sysfs
* @value: axis value for %EV_ABS
* @irq: Irq number in case of interrupt keys
+ * @wakeirq: Optional dedicated wake-up interrupt
*/
struct gpio_keys_button {
unsigned int code;
@@ -34,6 +35,7 @@ struct gpio_keys_button {
bool can_disable;
int value;
unsigned int irq;
+ unsigned int wakeirq;
};
/**
--
2.42.0
^ permalink raw reply
* [PATCH v3 1/2] dt-bindings: input: gpio-keys: Allow optional dedicated wakeirq
From: Tony Lindgren @ 2023-09-26 6:38 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-input,
devicetree, linux-kernel, linux-arm-kernel, Rob Herring
Allow configuring an optional dedicated wakeirq for gpio-keys that
some SoCs have.
Let's use the common interrupt naming "irq" and "wakeup" that we already
have in use for some drivers and subsystems like i2c framework.
Note that the gpio-keys interrupt property is optional. If only a gpio
property is specified, the driver tries to translate the gpio into an
interrupt.
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
Changes since v2:
- Fix indentation as noted by Rob's bot
- Add Reviewed-by from Rob
Changes since v1:
- Run make dt_binding_check on the binding
- Add better checks for interrupt-names as suggested by Rob, it is
now required if two interrupts are configured
- Add more decription entries
- Add a new example for key-wakeup
---
.../devicetree/bindings/input/gpio-keys.yaml | 41 ++++++++++++++++++-
1 file changed, 40 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/input/gpio-keys.yaml b/Documentation/devicetree/bindings/input/gpio-keys.yaml
--- a/Documentation/devicetree/bindings/input/gpio-keys.yaml
+++ b/Documentation/devicetree/bindings/input/gpio-keys.yaml
@@ -31,7 +31,23 @@ patternProperties:
maxItems: 1
interrupts:
- maxItems: 1
+ oneOf:
+ - items:
+ - description: Optional key interrupt or wakeup interrupt
+ - items:
+ - description: Key interrupt
+ - description: Wakeup interrupt
+
+ interrupt-names:
+ description:
+ Optional interrupt names, can be used to specify a separate dedicated
+ wake-up interrupt in addition to the gpio irq
+ oneOf:
+ - items:
+ - enum: [ irq, wakeup ]
+ - items:
+ - const: irq
+ - const: wakeup
label:
description: Descriptive name of the key.
@@ -97,6 +113,20 @@ patternProperties:
- required:
- gpios
+ allOf:
+ - if:
+ properties:
+ interrupts:
+ minItems: 2
+ required:
+ - interrupts
+ then:
+ properties:
+ interrupt-names:
+ minItems: 2
+ required:
+ - interrupt-names
+
dependencies:
wakeup-event-action: [ wakeup-source ]
linux,input-value: [ gpios ]
@@ -137,6 +167,15 @@ examples:
linux,code = <108>;
interrupts = <1 IRQ_TYPE_EDGE_FALLING>;
};
+
+ key-wakeup {
+ label = "GPIO Key WAKEUP";
+ linux,code = <143>;
+ interrupts-extended = <&intc 2 IRQ_TYPE_EDGE_FALLING>,
+ <&intc_wakeup 0 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "irq", "wakeup";
+ wakeup-source;
+ };
};
...
--
2.42.0
^ permalink raw reply
* Re: [PATCH v2] Input: axp20x-pek - avoid needless newline removal
From: Chen-Yu Tsai @ 2023-09-26 3:10 UTC (permalink / raw)
To: Kees Cook
Cc: Justin Stitt, Dmitry Torokhov, linux-input, linux-kernel,
linux-hardening
In-Reply-To: <202309251100.A187272A49@keescook>
On Tue, Sep 26, 2023 at 2:00 AM Kees Cook <keescook@chromium.org> wrote:
>
> On Mon, Sep 25, 2023 at 04:31:05AM +0000, Justin Stitt wrote:
> > This code is doing more work than it needs to.
> >
> > Before handing off `val_str` to `kstrtouint()` we are eagerly removing
> > any trailing newline which requires copying `buf`, validating it's
> > length and checking/replacing any potential newlines.
> >
> > kstrtouint() handles this implicitly:
> > kstrtouint ->
> > kstrotoull -> (documentation)
> > | /**
> > | * kstrtoull - convert a string to an unsigned long long
> > | * @s: The start of the string. The string must be null-terminated, and may also
> > | * include a single newline before its terminating null. The first character
> > | ...
> >
> > Let's remove the redundant functionality and let kstrtouint handle it.
> >
> > Link: https://github.com/KSPP/linux/issues/90
> > Cc: linux-hardening@vger.kernel.org
> > Suggested-by: Kees Cook <keescook@chromium.org>
> > Signed-off-by: Justin Stitt <justinstitt@google.com>
>
> This looks much cleaner. Thanks!
>
> Reviewed-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Chen-Yu Tsai <wens@csie.org>
^ permalink raw reply
* Re: Fwd: Kernel 6.4.4 breaks Elan Touchpad 1206
From: Bagas Sanjaya @ 2023-09-26 0:55 UTC (permalink / raw)
To: Dmitry Torokhov, Henrik Rydberg, Verot
Cc: Linux Input Devices, Linux Kernel Mailing List, Linux Regressions,
Linux Stable
In-Reply-To: <42bc8e02-5ee0-f1c8-610e-e16391e54ee2@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1386 bytes --]
On Tue, Jul 25, 2023 at 07:00:21AM +0700, Bagas Sanjaya wrote:
> Hi,
>
> I notice a regression report on Bugzilla [1]. Quoting from it:
>
> > Description:
> > When booting into Linux 6.4.4, system no longer recognizes touchpad input (confirmed with xinput). On the lts release, 6.1.39, the input is still recognized.
> >
> > Additional info:
> > * package version(s): Linux 6.4.4, 6.1.39
> > * Device: ELAN1206:00 04F3:30F1 Touchpad
> >
> > Steps to reproduce:
> > - Install 6.4.4 with Elan Touchpad 1206
> > - Reboot
> >
> > The issue might be related to bisected commit id: 7b63a88bb62ba2ddf5fcd956be85fe46624628b9
> > This is the only recent commit related to Elantech drivers I've noticed that may have broken the input.
>
> See Bugzilla for the full thread:
>
> To the reporter (Verot): Can you attach dmesg and lspci output?
>
> Anyway, I'm adding this regression to be tracked by regzbot:
>
> #regzbot introduced: 7b63a88bb62ba2 https://bugzilla.kernel.org/show_bug.cgi?id=217701
> #regzbot title: OOB protocol access fix breaks Elan Touchpad 1206
>
> Thanks.
>
> [1]: https://bugzilla.kernel.org/show_bug.cgi?id=217701
>
No reply from the reporter in regards to required bisection, thus:
#regzbot inconclusive: not bisected, reporter MIA
Sorry for inconvenience.
--
An old man doll... just what I always wanted! - Clara
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH 08/15] platform/x86/amd/pmf: Add support to update system state
From: kernel test robot @ 2023-09-25 21:42 UTC (permalink / raw)
To: Shyam Sundar S K, hdegoede, markgross, basavaraj.natikar, jikos,
benjamin.tissoires, alexander.deucher, christian.koenig,
Xinhui.Pan, airlied, daniel
Cc: oe-kbuild-all, Shyam Sundar S K, amd-gfx, platform-driver-x86,
dri-devel, Patil.Reddy, linux-input, mario.limonciello
In-Reply-To: <20230922175056.244940-9-Shyam-sundar.S-k@amd.com>
Hi Shyam,
kernel test robot noticed the following build warnings:
[auto build test WARNING on hid/for-next]
[also build test WARNING on linus/master v6.6-rc3 next-20230925]
[cannot apply to drm-misc/drm-misc-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Shyam-Sundar-S-K/platform-x86-amd-pmf-Add-PMF-TEE-interface/20230923-015418
base: https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git for-next
patch link: https://lore.kernel.org/r/20230922175056.244940-9-Shyam-sundar.S-k%40amd.com
patch subject: [PATCH 08/15] platform/x86/amd/pmf: Add support to update system state
reproduce: (https://download.01.org/0day-ci/archive/20230926/202309260515.5XbR6N0g-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202309260515.5XbR6N0g-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> Documentation/admin-guide/pmf.rst:16: WARNING: Unexpected indentation.
>> Documentation/admin-guide/pmf.rst: WARNING: document isn't included in any toctree
vim +16 Documentation/admin-guide/pmf.rst
13
14 Please add the following line(s) to
15 ``/etc/udev/rules.d/99-local.rules``::
> 16 DRIVERS=="amd-pmf", ACTION=="change", ENV{EVENT_ID}=="1", RUN+="/usr/bin/systemctl suspend"
17 DRIVERS=="amd-pmf", ACTION=="change", ENV{EVENT_ID}=="2", RUN+="/usr/bin/systemctl hibernate"
18 DRIVERS=="amd-pmf", ACTION=="change", ENV{EVENT_ID}=="3", RUN+="/bin/loginctl lock-sessions"
19
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* [PATCH] Input: i8042 - add quirk for Lenovo ThinkPad T14 Gen 1
From: Jonathan Denose @ 2023-09-25 21:33 UTC (permalink / raw)
To: linux-input
Cc: Jonathan Denose, Dmitry Torokhov, Hans de Goede, Huacai Chen,
Mattijs Korpershoek, Takashi Iwai, Werner Sembach, linux-kernel
The ThinkPad T14 Gen 1 touchpad works fine except that clicking
and dragging by tapping the touchpad or depressing the touchpad
do not work. Disabling PNP for controller setting discovery enables
click and drag without negatively impacting other touchpad features.
Signed-off-by: Jonathan Denose <jdenose@google.com>
---
drivers/input/serio/i8042-acpipnpio.h | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/input/serio/i8042-acpipnpio.h b/drivers/input/serio/i8042-acpipnpio.h
index 1724d6cb8649d..c487047d878cd 100644
--- a/drivers/input/serio/i8042-acpipnpio.h
+++ b/drivers/input/serio/i8042-acpipnpio.h
@@ -830,6 +830,14 @@ static const struct dmi_system_id i8042_dmi_quirk_table[] __initconst = {
},
.driver_data = (void *)(SERIO_QUIRK_RESET_ALWAYS)
},
+ {
+ /* Lenovo ThinkPad T14 Gen 1*/
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "20S0002UUS")
+ },
+ .driver_data = (void *)(SERIO_QUIRK_NOPNP)
+ },
{
/* Lenovo ThinkPad Twist S230u */
.matches = {
--
2.42.0.515.g380fc7ccd1-goog
^ permalink raw reply related
* Re: [PATCH v2] Input: axp20x-pek - avoid needless newline removal
From: Kees Cook @ 2023-09-25 18:00 UTC (permalink / raw)
To: Justin Stitt
Cc: Dmitry Torokhov, Chen-Yu Tsai, linux-input, linux-kernel,
linux-hardening
In-Reply-To: <20230925-strncpy-drivers-input-misc-axp20x-pek-c-v2-1-ff7abe8498d6@google.com>
On Mon, Sep 25, 2023 at 04:31:05AM +0000, Justin Stitt wrote:
> This code is doing more work than it needs to.
>
> Before handing off `val_str` to `kstrtouint()` we are eagerly removing
> any trailing newline which requires copying `buf`, validating it's
> length and checking/replacing any potential newlines.
>
> kstrtouint() handles this implicitly:
> kstrtouint ->
> kstrotoull -> (documentation)
> | /**
> | * kstrtoull - convert a string to an unsigned long long
> | * @s: The start of the string. The string must be null-terminated, and may also
> | * include a single newline before its terminating null. The first character
> | ...
>
> Let's remove the redundant functionality and let kstrtouint handle it.
>
> Link: https://github.com/KSPP/linux/issues/90
> Cc: linux-hardening@vger.kernel.org
> Suggested-by: Kees Cook <keescook@chromium.org>
> Signed-off-by: Justin Stitt <justinstitt@google.com>
This looks much cleaner. Thanks!
Reviewed-by: Kees Cook <keescook@chromium.org>
--
Kees Cook
^ permalink raw reply
* Re: [PATCH 13/15] platform/x86/amd/pmf: Add PMF-AMDGPU set interface
From: Mario Limonciello @ 2023-09-25 16:30 UTC (permalink / raw)
To: Deucher, Alexander, S-k, Shyam-sundar, hdegoede@redhat.com,
markgross@kernel.org, Natikar, Basavaraj, jikos@kernel.org,
benjamin.tissoires@redhat.com, Koenig, Christian, Pan, Xinhui,
airlied@gmail.com, daniel@ffwll.ch
Cc: amd-gfx@lists.freedesktop.org,
platform-driver-x86@vger.kernel.org,
dri-devel@lists.freedesktop.org, Patil Rajesh,
linux-input@vger.kernel.org
In-Reply-To: <BL1PR12MB51442D18C314B1E34B77DE2BF7FCA@BL1PR12MB5144.namprd12.prod.outlook.com>
On 9/25/2023 11:27, Deucher, Alexander wrote:
> [Public]
>
>> -----Original Message-----
>> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of
>> Shyam Sundar S K
>> Sent: Friday, September 22, 2023 1:51 PM
>> To: hdegoede@redhat.com; markgross@kernel.org; Natikar, Basavaraj
>> <Basavaraj.Natikar@amd.com>; jikos@kernel.org;
>> benjamin.tissoires@redhat.com; Deucher, Alexander
>> <Alexander.Deucher@amd.com>; Koenig, Christian
>> <Christian.Koenig@amd.com>; Pan, Xinhui <Xinhui.Pan@amd.com>;
>> airlied@gmail.com; daniel@ffwll.ch
>> Cc: S-k, Shyam-sundar <Shyam-sundar.S-k@amd.com>; amd-
>> gfx@lists.freedesktop.org; platform-driver-x86@vger.kernel.org; dri-
>> devel@lists.freedesktop.org; Patil Rajesh <Patil.Reddy@amd.com>; linux-
>> input@vger.kernel.org; Limonciello, Mario <Mario.Limonciello@amd.com>
>> Subject: [PATCH 13/15] platform/x86/amd/pmf: Add PMF-AMDGPU set
>> interface
>>
>> For the Smart PC Solution to fully work, it has to enact to the actions coming
>> from TA. Add the initial code path for set interface to AMDGPU.
>
> This seems to be limited to backlight at this point. What does setting or not setting the backlight level mean for the system when this framework is in place? What if a user manually changes the backlight level? Additional comments below.
>
It's also for the display count.
>>
>> Co-developed-by: Mario Limonciello <mario.limonciello@amd.com>
>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c | 21
>> +++++++++++++++++++++
>> drivers/platform/x86/amd/pmf/pmf.h | 2 ++
>> drivers/platform/x86/amd/pmf/tee-if.c | 19 +++++++++++++++++--
>> include/linux/amd-pmf-io.h | 1 +
>> 4 files changed, 41 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
>> index 232d11833ddc..5c567bff0548 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
>> @@ -68,3 +68,24 @@ int amd_pmf_get_gfx_data(struct amd_gpu_pmf_data
>> *pmf)
>> return 0;
>> }
>> EXPORT_SYMBOL_GPL(amd_pmf_get_gfx_data);
>> +
>> +int amd_pmf_set_gfx_data(struct amd_gpu_pmf_data *pmf) {
>> + struct drm_device *drm_dev = pci_get_drvdata(pmf->gpu_dev);
>> + struct amdgpu_device *adev = drm_to_adev(drm_dev);
>> + struct backlight_device *bd;
>> +
>> + if (!(adev->flags & AMD_IS_APU)) {
>> + DRM_ERROR("PMF-AMDGPU interface not supported\n");
>> + return -ENODEV;
>> + }
>> +
>> + bd = backlight_device_get_by_type(BACKLIGHT_RAW);
>> + if (!bd)
>> + return -ENODEV;
>> +
>> + backlight_device_set_brightness(bd, pmf->brightness);
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(amd_pmf_set_gfx_data);
>> diff --git a/drivers/platform/x86/amd/pmf/pmf.h
>> b/drivers/platform/x86/amd/pmf/pmf.h
>> index 9032df4ba48a..ce89cc0daa5a 100644
>> --- a/drivers/platform/x86/amd/pmf/pmf.h
>> +++ b/drivers/platform/x86/amd/pmf/pmf.h
>> @@ -73,6 +73,7 @@
>> #define PMF_POLICY_STT_SKINTEMP_APU 7
>> #define PMF_POLICY_STT_SKINTEMP_HS2 8
>> #define PMF_POLICY_SYSTEM_STATE 9
>> +#define PMF_POLICY_DISPLAY_BRIGHTNESS 12
>> #define PMF_POLICY_P3T 38
>>
>> /* TA macros */
>> @@ -480,6 +481,7 @@ enum ta_pmf_error_type { };
>>
>> struct pmf_action_table {
>> + unsigned long display_brightness;
>> enum system_state system_state;
>> unsigned long spl; /* in mW */
>> unsigned long sppt; /* in mW */
>> diff --git a/drivers/platform/x86/amd/pmf/tee-if.c
>> b/drivers/platform/x86/amd/pmf/tee-if.c
>> index 1608996654e8..eefffff83a4c 100644
>> --- a/drivers/platform/x86/amd/pmf/tee-if.c
>> +++ b/drivers/platform/x86/amd/pmf/tee-if.c
>> @@ -79,10 +79,10 @@ static int amd_pmf_update_uevents(struct
>> amd_pmf_dev *dev, u16 event)
>> return 0;
>> }
>>
>> -static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct
>> ta_pmf_enact_result *out)
>> +static int amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct
>> +ta_pmf_enact_result *out)
>> {
>> u32 val, event = 0;
>> - int idx;
>> + int idx, ret;
>>
>> for (idx = 0; idx < out->actions_count; idx++) {
>> val = out->actions_list[idx].value;
>> @@ -160,8 +160,23 @@ static void amd_pmf_apply_policies(struct
>> amd_pmf_dev *dev, struct ta_pmf_enact_
>> dev->prev_data->system_state = 0;
>> }
>> break;
>> +
>> + case PMF_POLICY_DISPLAY_BRIGHTNESS:
>> + ret = amd_pmf_get_gfx_data(&dev->gfx_data);
>> + if (ret)
>> + return ret;
>> +
>> + dev->prev_data->display_brightness = dev-
>>> gfx_data.brightness;
>
> Are we using standardized units for the brightness? On the GPU side, we align with the standard blacklight interface, but internally, the driver has to convert units depending on the type of backlight controller used on the platform. Presumably PMF does something similar?
>
> Alex
>
>> + if (dev->prev_data->display_brightness != val) {
>> + dev->gfx_data.brightness = val;
>> + amd_pmf_set_gfx_data(&dev->gfx_data);
>> + dev_dbg(dev->dev, "update
>> DISPLAY_BRIGHTNESS : %d\n", val);
>> + }
>> + break;
>> }
>> }
>> +
>> + return 0;
>> }
>>
>> static int amd_pmf_invoke_cmd_enact(struct amd_pmf_dev *dev) diff --git
>> a/include/linux/amd-pmf-io.h b/include/linux/amd-pmf-io.h index
>> a2d4af231362..ecae387ddaa6 100644
>> --- a/include/linux/amd-pmf-io.h
>> +++ b/include/linux/amd-pmf-io.h
>> @@ -25,4 +25,5 @@ struct amd_gpu_pmf_data { };
>>
>> int amd_pmf_get_gfx_data(struct amd_gpu_pmf_data *pmf);
>> +int amd_pmf_set_gfx_data(struct amd_gpu_pmf_data *pmf);
>> #endif
>> --
>> 2.25.1
>
^ permalink raw reply
* RE: [PATCH 13/15] platform/x86/amd/pmf: Add PMF-AMDGPU set interface
From: Deucher, Alexander @ 2023-09-25 16:27 UTC (permalink / raw)
To: S-k, Shyam-sundar, hdegoede@redhat.com, markgross@kernel.org,
Natikar, Basavaraj, jikos@kernel.org,
benjamin.tissoires@redhat.com, Koenig, Christian, Pan, Xinhui,
airlied@gmail.com, daniel@ffwll.ch
Cc: S-k, Shyam-sundar, amd-gfx@lists.freedesktop.org,
platform-driver-x86@vger.kernel.org,
dri-devel@lists.freedesktop.org, Patil Rajesh,
linux-input@vger.kernel.org, Limonciello, Mario
In-Reply-To: <20230922175056.244940-14-Shyam-sundar.S-k@amd.com>
[Public]
> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of
> Shyam Sundar S K
> Sent: Friday, September 22, 2023 1:51 PM
> To: hdegoede@redhat.com; markgross@kernel.org; Natikar, Basavaraj
> <Basavaraj.Natikar@amd.com>; jikos@kernel.org;
> benjamin.tissoires@redhat.com; Deucher, Alexander
> <Alexander.Deucher@amd.com>; Koenig, Christian
> <Christian.Koenig@amd.com>; Pan, Xinhui <Xinhui.Pan@amd.com>;
> airlied@gmail.com; daniel@ffwll.ch
> Cc: S-k, Shyam-sundar <Shyam-sundar.S-k@amd.com>; amd-
> gfx@lists.freedesktop.org; platform-driver-x86@vger.kernel.org; dri-
> devel@lists.freedesktop.org; Patil Rajesh <Patil.Reddy@amd.com>; linux-
> input@vger.kernel.org; Limonciello, Mario <Mario.Limonciello@amd.com>
> Subject: [PATCH 13/15] platform/x86/amd/pmf: Add PMF-AMDGPU set
> interface
>
> For the Smart PC Solution to fully work, it has to enact to the actions coming
> from TA. Add the initial code path for set interface to AMDGPU.
This seems to be limited to backlight at this point. What does setting or not setting the backlight level mean for the system when this framework is in place? What if a user manually changes the backlight level? Additional comments below.
>
> Co-developed-by: Mario Limonciello <mario.limonciello@amd.com>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c | 21
> +++++++++++++++++++++
> drivers/platform/x86/amd/pmf/pmf.h | 2 ++
> drivers/platform/x86/amd/pmf/tee-if.c | 19 +++++++++++++++++--
> include/linux/amd-pmf-io.h | 1 +
> 4 files changed, 41 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
> index 232d11833ddc..5c567bff0548 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
> @@ -68,3 +68,24 @@ int amd_pmf_get_gfx_data(struct amd_gpu_pmf_data
> *pmf)
> return 0;
> }
> EXPORT_SYMBOL_GPL(amd_pmf_get_gfx_data);
> +
> +int amd_pmf_set_gfx_data(struct amd_gpu_pmf_data *pmf) {
> + struct drm_device *drm_dev = pci_get_drvdata(pmf->gpu_dev);
> + struct amdgpu_device *adev = drm_to_adev(drm_dev);
> + struct backlight_device *bd;
> +
> + if (!(adev->flags & AMD_IS_APU)) {
> + DRM_ERROR("PMF-AMDGPU interface not supported\n");
> + return -ENODEV;
> + }
> +
> + bd = backlight_device_get_by_type(BACKLIGHT_RAW);
> + if (!bd)
> + return -ENODEV;
> +
> + backlight_device_set_brightness(bd, pmf->brightness);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(amd_pmf_set_gfx_data);
> diff --git a/drivers/platform/x86/amd/pmf/pmf.h
> b/drivers/platform/x86/amd/pmf/pmf.h
> index 9032df4ba48a..ce89cc0daa5a 100644
> --- a/drivers/platform/x86/amd/pmf/pmf.h
> +++ b/drivers/platform/x86/amd/pmf/pmf.h
> @@ -73,6 +73,7 @@
> #define PMF_POLICY_STT_SKINTEMP_APU 7
> #define PMF_POLICY_STT_SKINTEMP_HS2 8
> #define PMF_POLICY_SYSTEM_STATE 9
> +#define PMF_POLICY_DISPLAY_BRIGHTNESS 12
> #define PMF_POLICY_P3T 38
>
> /* TA macros */
> @@ -480,6 +481,7 @@ enum ta_pmf_error_type { };
>
> struct pmf_action_table {
> + unsigned long display_brightness;
> enum system_state system_state;
> unsigned long spl; /* in mW */
> unsigned long sppt; /* in mW */
> diff --git a/drivers/platform/x86/amd/pmf/tee-if.c
> b/drivers/platform/x86/amd/pmf/tee-if.c
> index 1608996654e8..eefffff83a4c 100644
> --- a/drivers/platform/x86/amd/pmf/tee-if.c
> +++ b/drivers/platform/x86/amd/pmf/tee-if.c
> @@ -79,10 +79,10 @@ static int amd_pmf_update_uevents(struct
> amd_pmf_dev *dev, u16 event)
> return 0;
> }
>
> -static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct
> ta_pmf_enact_result *out)
> +static int amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct
> +ta_pmf_enact_result *out)
> {
> u32 val, event = 0;
> - int idx;
> + int idx, ret;
>
> for (idx = 0; idx < out->actions_count; idx++) {
> val = out->actions_list[idx].value;
> @@ -160,8 +160,23 @@ static void amd_pmf_apply_policies(struct
> amd_pmf_dev *dev, struct ta_pmf_enact_
> dev->prev_data->system_state = 0;
> }
> break;
> +
> + case PMF_POLICY_DISPLAY_BRIGHTNESS:
> + ret = amd_pmf_get_gfx_data(&dev->gfx_data);
> + if (ret)
> + return ret;
> +
> + dev->prev_data->display_brightness = dev-
> >gfx_data.brightness;
Are we using standardized units for the brightness? On the GPU side, we align with the standard blacklight interface, but internally, the driver has to convert units depending on the type of backlight controller used on the platform. Presumably PMF does something similar?
Alex
> + if (dev->prev_data->display_brightness != val) {
> + dev->gfx_data.brightness = val;
> + amd_pmf_set_gfx_data(&dev->gfx_data);
> + dev_dbg(dev->dev, "update
> DISPLAY_BRIGHTNESS : %d\n", val);
> + }
> + break;
> }
> }
> +
> + return 0;
> }
>
> static int amd_pmf_invoke_cmd_enact(struct amd_pmf_dev *dev) diff --git
> a/include/linux/amd-pmf-io.h b/include/linux/amd-pmf-io.h index
> a2d4af231362..ecae387ddaa6 100644
> --- a/include/linux/amd-pmf-io.h
> +++ b/include/linux/amd-pmf-io.h
> @@ -25,4 +25,5 @@ struct amd_gpu_pmf_data { };
>
> int amd_pmf_get_gfx_data(struct amd_gpu_pmf_data *pmf);
> +int amd_pmf_set_gfx_data(struct amd_gpu_pmf_data *pmf);
> #endif
> --
> 2.25.1
^ permalink raw reply
* Re: [PATCH v2] dt-bindings: input: touchscreen: goodix: clarify irq-gpios misleading text
From: Rob Herring @ 2023-09-25 16:25 UTC (permalink / raw)
To: Luca Ceresoli
Cc: linux-input, linux-kernel, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Thomas Petazzoni, Jeff LaBundy, devicetree,
Conor Dooley
In-Reply-To: <20230925032208.11698-1-luca.ceresoli@bootlin.com>
On Mon, 25 Sep 2023 05:22:08 +0200, Luca Ceresoli wrote:
> The irq-gpios description misleading, apparently saying that driving the
> IRQ GPIO resets the device, which is even more puzzling as there is a reset
> GPIO as well.
>
> In reality the IRQ pin can be driven during the reset sequence to configure
> the client address, as it becomes clear after checking both the datasheet
> and the driver code. Improve the text to clarify that.
>
> Also rephrase to remove reference to the driver, which is not appropriate
> in the bindings.
>
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
>
> ---
>
> Changed in v2:
> - reworded to clarify even further
> ---
> .../devicetree/bindings/input/touchscreen/goodix.yaml | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox