| Message ID | 20240621-scmi-mailbox-v1-v1-1-8ed450735f46@nxp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | firmware: arm_scmi: introduce max-rx-timeout-ms property | expand |
On Fri, Jun 21, 2024 at 08:46:57PM +0800, Peng Fan (OSS) wrote: > From: Peng Fan <peng.fan@nxp.com> > > System Controller Management Interface(SCMI) firmwares might have > different designs by SCMI firmware developers. So the maximum receive > channel timeout value might also varies in the various designs. > > So introduce property mbox-rx-timeout-ms to let each platform could > set its own timeout value in device tree. > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > --- > Documentation/devicetree/bindings/firmware/arm,scmi.yaml | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml > index 4d823f3b1f0e..d6cc2bf4c819 100644 > --- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml > +++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml > @@ -121,6 +121,12 @@ properties: > atomic mode of operation, even if requested. > default: 0 > > + max-rx-timeout-ms: > + description: > + An optional time value, expressed in milliseconds, representing, on this > + platform, the mailbox maximum timeout value for receive channel. "on this platform"? Doesn't every property apply to the given platform? > + default: 0 0 means no timeout or response is instant? > + > arm,smc-id: > $ref: /schemas/types.yaml#/definitions/uint32 > description: > > -- > 2.37.1 >
> Subject: Re: [PATCH 1/2] dt-bindings: firmware: arm,scmi: introduce > property mbox-rx-timeout-ms > > On Fri, Jun 21, 2024 at 08:46:57PM +0800, Peng Fan (OSS) wrote: > > From: Peng Fan <peng.fan@nxp.com> > > > > System Controller Management Interface(SCMI) firmwares might > have > > different designs by SCMI firmware developers. So the maximum > receive > > channel timeout value might also varies in the various designs. > > > > So introduce property mbox-rx-timeout-ms to let each platform could > > set its own timeout value in device tree. > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > > --- > > Documentation/devicetree/bindings/firmware/arm,scmi.yaml | 6 > ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git > a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml > > b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml > > index 4d823f3b1f0e..d6cc2bf4c819 100644 > > --- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml > > +++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml > > @@ -121,6 +121,12 @@ properties: > > atomic mode of operation, even if requested. > > default: 0 > > > > + max-rx-timeout-ms: > > + description: > > + An optional time value, expressed in milliseconds, representing, > on this > > + platform, the mailbox maximum timeout value for receive > channel. > > "on this platform"? Doesn't every property apply to the given platform? Yeah, apply to all the use mailbox. > > > + default: 0 > > 0 means no timeout or response is instant? I should use 30ms same as what the driver currently use. Thanks, Peng. > > > + > > arm,smc-id: > > $ref: /schemas/types.yaml#/definitions/uint32 > > description: > > > > -- > > 2.37.1 > >
On Thu, Jun 27, 2024 at 11:17:49PM +0000, Peng Fan wrote: > > Subject: Re: [PATCH 1/2] dt-bindings: firmware: arm,scmi: introduce > > property mbox-rx-timeout-ms > > > > On Fri, Jun 21, 2024 at 08:46:57PM +0800, Peng Fan (OSS) wrote: > > > From: Peng Fan <peng.fan@nxp.com> > > > > > > System Controller Management Interface(SCMI) firmwares might > > have > > > different designs by SCMI firmware developers. So the maximum > > receive > > > channel timeout value might also varies in the various designs. > > > > > > So introduce property mbox-rx-timeout-ms to let each platform could > > > set its own timeout value in device tree. > > > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > > > --- > > > Documentation/devicetree/bindings/firmware/arm,scmi.yaml | 6 > > ++++++ > > > 1 file changed, 6 insertions(+) > > > > > > diff --git > > a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml > > > b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml > > > index 4d823f3b1f0e..d6cc2bf4c819 100644 > > > --- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml > > > +++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml > > > @@ -121,6 +121,12 @@ properties: > > > atomic mode of operation, even if requested. > > > default: 0 > > > > > > + max-rx-timeout-ms: > > > + description: > > > + An optional time value, expressed in milliseconds, representing, > > on this > > > + platform, the mailbox maximum timeout value for receive > > channel. > > > > "on this platform"? Doesn't every property apply to the given platform? > > Yeah, apply to all the use mailbox. > > > > > > + default: 0 > > > > 0 means no timeout or response is instant? > > I should use 30ms same as what the driver currently use. > That sounds very wrong to me. The binding is independent of current driver behaviour. How the driver handles the case of default 0 value is different from what the default value in the DT means IMO. You can't just set a default value in the DT binding based on the current driver setting. We can always say since it is optional, absence of it is what driver handles as 30ms. 0ms is impossible or incorrect value as transport involves some delay even if it is in terms of uS. So I prefer to set a value of > 0 in DT and make that a requirement. -- Regards, Sudeep
Hi Rob, Sudeep > Subject: Re: [PATCH 1/2] dt-bindings: firmware: arm,scmi: introduce > property mbox-rx-timeout-ms > > On Thu, Jun 27, 2024 at 11:17:49PM +0000, Peng Fan wrote: > > > Subject: Re: [PATCH 1/2] dt-bindings: firmware: arm,scmi: > introduce > > > property mbox-rx-timeout-ms > > > > > > On Fri, Jun 21, 2024 at 08:46:57PM +0800, Peng Fan (OSS) wrote: > > > > From: Peng Fan <peng.fan@nxp.com> > > > > > > > > System Controller Management Interface(SCMI) firmwares might > > > have > > > > different designs by SCMI firmware developers. So the maximum > > > receive > > > > channel timeout value might also varies in the various designs. > > > > > > > > So introduce property mbox-rx-timeout-ms to let each platform > > > > could set its own timeout value in device tree. > > > > > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > > > > --- > > > > Documentation/devicetree/bindings/firmware/arm,scmi.yaml | 6 > > > ++++++ > > > > 1 file changed, 6 insertions(+) > > > > > > > > diff --git > > > a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml > > > > b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml > > > > index 4d823f3b1f0e..d6cc2bf4c819 100644 > > > > --- > a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml > > > > +++ > b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml > > > > @@ -121,6 +121,12 @@ properties: > > > > atomic mode of operation, even if requested. > > > > default: 0 > > > > > > > > + max-rx-timeout-ms: > > > > + description: > > > > + An optional time value, expressed in milliseconds, > > > > + representing, > > > on this > > > > + platform, the mailbox maximum timeout value for receive > > > channel. > > > > > > "on this platform"? Doesn't every property apply to the given > platform? > > > > Yeah, apply to all the use mailbox. > > > > > > > > > + default: 0 > > > > > > 0 means no timeout or response is instant? > > > > I should use 30ms same as what the driver currently use. > > > > That sounds very wrong to me. The binding is independent of current > driver behaviour. How the driver handles the case of default 0 value is > different from what the default value in the DT means IMO. You can't > just set a default value in the DT binding based on the current driver > setting. > > We can always say since it is optional, absence of it is what driver > handles as 30ms. 0ms is impossible or incorrect value as transport > involves some delay even if it is in terms of uS. So I prefer to set a value > of > 0 in DT and make that a requirement. How about this? max-rx-timeout-ms: $ref: /schemas/types.yaml#/definitions/uint32 description: An optional time value, expressed in milliseconds, representing the mailbox maximum timeout value for receive channel. minimum: 1 maximum: 5000 or? max-rx-timeout-ms: description: An optional time value, expressed in milliseconds, representing the mailbox maximum timeout value for receive channel. The value should be a non-zero value if set. Thanks, Peng. > > -- > Regards, > Sudeep
diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml index 4d823f3b1f0e..d6cc2bf4c819 100644 --- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml +++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml @@ -121,6 +121,12 @@ properties: atomic mode of operation, even if requested. default: 0 + max-rx-timeout-ms: + description: + An optional time value, expressed in milliseconds, representing, on this + platform, the mailbox maximum timeout value for receive channel. + default: 0 + arm,smc-id: $ref: /schemas/types.yaml#/definitions/uint32 description: