[PATCH v3 1/2] dt-bindings: google,cros-ec-keyb: add has-fn-map prop

Fabio Baltieri posted 2 patches 1 month, 1 week ago
There is a newer version of this series
[PATCH v3 1/2] dt-bindings: google,cros-ec-keyb: add has-fn-map prop
Posted by Fabio Baltieri 1 month, 1 week ago
Add binding documentation for the has-fn-map property.

Signed-off-by: Fabio Baltieri <fabiobaltieri@chromium.org>
---
 .../devicetree/bindings/input/google,cros-ec-keyb.yaml    | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/input/google,cros-ec-keyb.yaml b/Documentation/devicetree/bindings/input/google,cros-ec-keyb.yaml
index fefaaf46a240..fa24b1cbc788 100644
--- a/Documentation/devicetree/bindings/input/google,cros-ec-keyb.yaml
+++ b/Documentation/devicetree/bindings/input/google,cros-ec-keyb.yaml
@@ -44,6 +44,14 @@ properties:
       where the lower 16 bits are reserved. This property is specified only
       when the keyboard has a custom design for the top row keys.
 
+  google,has-fn-map:
+    description: |
+      The keymap has function key layer. This allows defining an extra set of
+      codes that are sent if a key is pressed while the KEY_FN is held pressed
+      as well. The function codes have to be defined in the linux,keymap
+      property with an offset of keypad,num-rows from the normal ones.
+    type: boolean
+
 dependencies:
   function-row-physmap: [ 'linux,keymap' ]
   google,needs-ghost-filter: [ 'linux,keymap' ]
-- 
2.52.0.351.gbe84eed79e-goog
Re: [PATCH v3 1/2] dt-bindings: google,cros-ec-keyb: add has-fn-map prop
Posted by Krzysztof Kozlowski 1 month ago
On Wed, Dec 31, 2025 at 02:35:37PM +0000, Fabio Baltieri wrote:
> Add binding documentation for the has-fn-map property.
> 
> Signed-off-by: Fabio Baltieri <fabiobaltieri@chromium.org>
> ---
>  .../devicetree/bindings/input/google,cros-ec-keyb.yaml    | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/input/google,cros-ec-keyb.yaml b/Documentation/devicetree/bindings/input/google,cros-ec-keyb.yaml
> index fefaaf46a240..fa24b1cbc788 100644
> --- a/Documentation/devicetree/bindings/input/google,cros-ec-keyb.yaml
> +++ b/Documentation/devicetree/bindings/input/google,cros-ec-keyb.yaml
> @@ -44,6 +44,14 @@ properties:
>        where the lower 16 bits are reserved. This property is specified only
>        when the keyboard has a custom design for the top row keys.
>  
> +  google,has-fn-map:
> +    description: |
> +      The keymap has function key layer. This allows defining an extra set of
> +      codes that are sent if a key is pressed while the KEY_FN is held pressed
> +      as well. The function codes have to be defined in the linux,keymap
> +      property with an offset of keypad,num-rows from the normal ones.
> +    type: boolean

You still did not answer to my previous question, why this is not
deducible from the key map (presence of KEY_FN in the map).

Best regards,
Krzysztof
Re: [PATCH v3 1/2] dt-bindings: google,cros-ec-keyb: add has-fn-map prop
Posted by Fabio Baltieri 1 month ago
On Mon, Jan 05, 2026 at 08:52:56AM +0100, Krzysztof Kozlowski wrote:
> On Wed, Dec 31, 2025 at 02:35:37PM +0000, Fabio Baltieri wrote:
> > Add binding documentation for the has-fn-map property.
> > 
> > Signed-off-by: Fabio Baltieri <fabiobaltieri@chromium.org>
> > ---
> >  .../devicetree/bindings/input/google,cros-ec-keyb.yaml    | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/input/google,cros-ec-keyb.yaml b/Documentation/devicetree/bindings/input/google,cros-ec-keyb.yaml
> > index fefaaf46a240..fa24b1cbc788 100644
> > --- a/Documentation/devicetree/bindings/input/google,cros-ec-keyb.yaml
> > +++ b/Documentation/devicetree/bindings/input/google,cros-ec-keyb.yaml
> > @@ -44,6 +44,14 @@ properties:
> >        where the lower 16 bits are reserved. This property is specified only
> >        when the keyboard has a custom design for the top row keys.
> >  
> > +  google,has-fn-map:
> > +    description: |
> > +      The keymap has function key layer. This allows defining an extra set of
> > +      codes that are sent if a key is pressed while the KEY_FN is held pressed
> > +      as well. The function codes have to be defined in the linux,keymap
> > +      property with an offset of keypad,num-rows from the normal ones.
> > +    type: boolean
> 
> You still did not answer to my previous question, why this is not
> deducible from the key map (presence of KEY_FN in the map).

The driver behaves differently with the fn layer is present, has to make
extra space for the extra codes and enable the logic to use it. I can
certainly detect it in runtime, would have to always allocate the extra
space even if not needed and check not only that there is an FN key but
if there's anything in the second half of the map.

I'm not overly enthusiastic about it, it's a bit wasteful on memory
(probably no big deal, half a kb of RAM I guess) and somewhat less
defensive to misconfigurations and in general I don't like the new logic
to be enabled magically, as a side effect. It'd be extra complexity for
the sake of saving one boolean property, but sure if you think that's
the way to go then I guess I can implement it that way.
Re: [PATCH v3 1/2] dt-bindings: google,cros-ec-keyb: add has-fn-map prop
Posted by Krzysztof Kozlowski 1 month ago
On Mon, Jan 05, 2026 at 10:55:51AM +0000, Fabio Baltieri wrote:
> On Mon, Jan 05, 2026 at 08:52:56AM +0100, Krzysztof Kozlowski wrote:
> > On Wed, Dec 31, 2025 at 02:35:37PM +0000, Fabio Baltieri wrote:
> > > Add binding documentation for the has-fn-map property.
> > > 
> > > Signed-off-by: Fabio Baltieri <fabiobaltieri@chromium.org>
> > > ---
> > >  .../devicetree/bindings/input/google,cros-ec-keyb.yaml    | 8 ++++++++
> > >  1 file changed, 8 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/input/google,cros-ec-keyb.yaml b/Documentation/devicetree/bindings/input/google,cros-ec-keyb.yaml
> > > index fefaaf46a240..fa24b1cbc788 100644
> > > --- a/Documentation/devicetree/bindings/input/google,cros-ec-keyb.yaml
> > > +++ b/Documentation/devicetree/bindings/input/google,cros-ec-keyb.yaml
> > > @@ -44,6 +44,14 @@ properties:
> > >        where the lower 16 bits are reserved. This property is specified only
> > >        when the keyboard has a custom design for the top row keys.
> > >  
> > > +  google,has-fn-map:
> > > +    description: |
> > > +      The keymap has function key layer. This allows defining an extra set of
> > > +      codes that are sent if a key is pressed while the KEY_FN is held pressed
> > > +      as well. The function codes have to be defined in the linux,keymap
> > > +      property with an offset of keypad,num-rows from the normal ones.
> > > +    type: boolean
> > 
> > You still did not answer to my previous question, why this is not
> > deducible from the key map (presence of KEY_FN in the map).
> 
> The driver behaves differently with the fn layer is present, has to make
> extra space for the extra codes and enable the logic to use it. I can
> certainly detect it in runtime, would have to always allocate the extra
> space even if not needed and check not only that there is an FN key but
> if there's anything in the second half of the map.
> 
> I'm not overly enthusiastic about it, it's a bit wasteful on memory
> (probably no big deal, half a kb of RAM I guess) and somewhat less
> defensive to misconfigurations and in general I don't like the new logic
> to be enabled magically, as a side effect. It'd be extra complexity for
> the sake of saving one boolean property, but sure if you think that's
> the way to go then I guess I can implement it that way.

Driver logic is not an argument here, we don't care about it. You should
answer why presence of google,has-fn-map in DT makes sense when none of
the keymaps has KEY_FN. Why this is a valid and desired configuration?

Best regards,
Krzysztof
Re: [PATCH v3 1/2] dt-bindings: google,cros-ec-keyb: add has-fn-map prop
Posted by Fabio Baltieri 1 month ago
On Tue, Jan 06, 2026 at 08:27:10AM +0100, Krzysztof Kozlowski wrote:
> On Mon, Jan 05, 2026 at 10:55:51AM +0000, Fabio Baltieri wrote:
> > On Mon, Jan 05, 2026 at 08:52:56AM +0100, Krzysztof Kozlowski wrote:
> > > On Wed, Dec 31, 2025 at 02:35:37PM +0000, Fabio Baltieri wrote:
> > > > Add binding documentation for the has-fn-map property.
> > > > 
> > > > Signed-off-by: Fabio Baltieri <fabiobaltieri@chromium.org>
> > > > ---
> > > >  .../devicetree/bindings/input/google,cros-ec-keyb.yaml    | 8 ++++++++
> > > >  1 file changed, 8 insertions(+)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/input/google,cros-ec-keyb.yaml b/Documentation/devicetree/bindings/input/google,cros-ec-keyb.yaml
> > > > index fefaaf46a240..fa24b1cbc788 100644
> > > > --- a/Documentation/devicetree/bindings/input/google,cros-ec-keyb.yaml
> > > > +++ b/Documentation/devicetree/bindings/input/google,cros-ec-keyb.yaml
> > > > @@ -44,6 +44,14 @@ properties:
> > > >        where the lower 16 bits are reserved. This property is specified only
> > > >        when the keyboard has a custom design for the top row keys.
> > > >  
> > > > +  google,has-fn-map:
> > > > +    description: |
> > > > +      The keymap has function key layer. This allows defining an extra set of
> > > > +      codes that are sent if a key is pressed while the KEY_FN is held pressed
> > > > +      as well. The function codes have to be defined in the linux,keymap
> > > > +      property with an offset of keypad,num-rows from the normal ones.
> > > > +    type: boolean
> > > 
> > > You still did not answer to my previous question, why this is not
> > > deducible from the key map (presence of KEY_FN in the map).
> > 
> > The driver behaves differently with the fn layer is present, has to make
> > extra space for the extra codes and enable the logic to use it. I can
> > certainly detect it in runtime, would have to always allocate the extra
> > space even if not needed and check not only that there is an FN key but
> > if there's anything in the second half of the map.
> > 
> > I'm not overly enthusiastic about it, it's a bit wasteful on memory
> > (probably no big deal, half a kb of RAM I guess) and somewhat less
> > defensive to misconfigurations and in general I don't like the new logic
> > to be enabled magically, as a side effect. It'd be extra complexity for
> > the sake of saving one boolean property, but sure if you think that's
> > the way to go then I guess I can implement it that way.
> 
> Driver logic is not an argument here, we don't care about it. You should
> answer why presence of google,has-fn-map in DT makes sense when none of
> the keymaps has KEY_FN. Why this is a valid and desired configuration?
> 

Think I answered that, it felt like a clarer design to me. Anyway I got
the message will send a v4 with runtime detection, it's certainly a
valid option and works out for me.