On Tue, 20 Sep 2022 13:47:36 -0400, Asmaa Mnebhi wrote:
> In the latest version of the i2c-mlxbf.c driver, the "Smbus block"
> resource was broken down to 3 separate resources "Smbus timer",
> "Smbus master" and "Smbus slave" to accommodate for BlueField-3
> SoC registers' changes.
>
> Reviewed-by: Khalil Blaiech <[email protected]>
> Signed-off-by: Asmaa Mnebhi <[email protected]>
> ---
> .../bindings/i2c/mellanox,i2c-mlxbf.yaml | 48 ++++++++++++++-----
> 1 file changed, 36 insertions(+), 12 deletions(-)
>
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
dtschema/dtc warnings/errors:
Error: Documentation/devicetree/bindings/i2c/mellanox,i2c-mlxbf.example.dts:26.19-20 syntax error
FATAL ERROR: Unable to parse input tree
make[1]: *** [scripts/Makefile.lib:384: Documentation/devicetree/bindings/i2c/mellanox,i2c-mlxbf.example.dtb] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1420: dt_binding_check] Error 2
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/patch/
This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit.
I have a question for you and Wolfram, we don’t use device trees and are not planning to use device trees; we only use ACPI tables. But I think when Khalil submitted the first version of the i2c-mlxbf.c driver, it was requested from him to add devicetree support. Do you know why? Is it possible to remove the device tree support and so this doc? or is devicetree support a requirement regardless of the actual implementation?
-----Original Message-----
From: Krzysztof Kozlowski <[email protected]>
Sent: Wednesday, September 21, 2022 2:55 AM
To: Asmaa Mnebhi <[email protected]>
Cc: Wolfram Sang <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; Khalil Blaiech <[email protected]>
Subject: Re: [PATCH v5 8/8] i2c: i2c-mlxbf.c: Update binding devicetree
On Tue, 20 Sep 2022 13:47:36 -0400, Asmaa Mnebhi wrote:
> In the latest version of the i2c-mlxbf.c driver, the "Smbus block"
> resource was broken down to 3 separate resources "Smbus timer", "Smbus
> master" and "Smbus slave" to accommodate for BlueField-3 SoC
> registers' changes.
>
> Reviewed-by: Khalil Blaiech <[email protected]>
> Signed-off-by: Asmaa Mnebhi <[email protected]>
> ---
> .../bindings/i2c/mellanox,i2c-mlxbf.yaml | 48 ++++++++++++++-----
> 1 file changed, 36 insertions(+), 12 deletions(-)
>
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
dtschema/dtc warnings/errors:
Error: Documentation/devicetree/bindings/i2c/mellanox,i2c-mlxbf.example.dts:26.19-20 syntax error FATAL ERROR: Unable to parse input tree
make[1]: *** [scripts/Makefile.lib:384: Documentation/devicetree/bindings/i2c/mellanox,i2c-mlxbf.example.dtb] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1420: dt_binding_check] Error 2
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/patch/
This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1.
If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit.
Hi,
> I have a question for you and Wolfram, we don’t use device trees and
> are not planning to use device trees; we only use ACPI tables. But I
> think when Khalil submitted the first version of the i2c-mlxbf.c
> driver, it was requested from him to add devicetree support. Do you
> know why? Is it possible to remove the device tree support and so this
> doc? or is devicetree support a requirement regardless of the actual
> implementation?
The first version sent from Khalil to the public I2C mailing list already
had DT bindings [1]. I don't see a sign of someone of the public list
requesting DT bindings. Maybe it was company internal?
Technically, there is no requirement to support DT, especially since you
have working ACPI. I don't know the process, though, of removing DT
support. You would basically need to be sure that no user made use of
the DT bindings introduced before. I don't know to what degree you can
assume that.
Maybe the DT list has more to add here?
Happy hacking,
Wolfram
[1] http://patchwork.ozlabs.org/project/linux-i2c/list/?series=73827&state=*
Thanks for your reply Wolfram. All customers using BlueField hardware need to install our internal Firmware (proprietary) before booting any customized OS so they will always use ACPI tables. So I think it is safe to remove it. Any feedback from the DT list would be greatly appreciated!
-----Original Message-----
From: Wolfram Sang <[email protected]>
Sent: Wednesday, September 21, 2022 4:02 PM
To: Asmaa Mnebhi <[email protected]>
Cc: Krzysztof Kozlowski <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; Khalil Blaiech <[email protected]>
Subject: How to remove DT support from a driver? (was Re: [PATCH v5 8/8] i2c: i2c-mlxbf.c: Update binding devicetree)
Hi,
> I have a question for you and Wolfram, we don’t use device trees and
> are not planning to use device trees; we only use ACPI tables. But I
> think when Khalil submitted the first version of the i2c-mlxbf.c
> driver, it was requested from him to add devicetree support. Do you
> know why? Is it possible to remove the device tree support and so this
> doc? or is devicetree support a requirement regardless of the actual
> implementation?
The first version sent from Khalil to the public I2C mailing list already had DT bindings [1]. I don't see a sign of someone of the public list requesting DT bindings. Maybe it was company internal?
Technically, there is no requirement to support DT, especially since you have working ACPI. I don't know the process, though, of removing DT support. You would basically need to be sure that no user made use of the DT bindings introduced before. I don't know to what degree you can assume that.
Maybe the DT list has more to add here?
Happy hacking,
Wolfram
[1] http://patchwork.ozlabs.org/project/linux-i2c/list/?series=73827&state=*
On Wed, Sep 21, 2022 at 10:01:59PM +0200, Wolfram Sang wrote:
> Hi,
>
> > I have a question for you and Wolfram, we don’t use device trees and
> > are not planning to use device trees; we only use ACPI tables. But I
> > think when Khalil submitted the first version of the i2c-mlxbf.c
> > driver, it was requested from him to add devicetree support. Do you
> > know why? Is it possible to remove the device tree support and so this
> > doc? or is devicetree support a requirement regardless of the actual
> > implementation?
>
> The first version sent from Khalil to the public I2C mailing list already
> had DT bindings [1]. I don't see a sign of someone of the public list
> requesting DT bindings. Maybe it was company internal?
>
> Technically, there is no requirement to support DT, especially since you
> have working ACPI. I don't know the process, though, of removing DT
> support. You would basically need to be sure that no user made use of
> the DT bindings introduced before. I don't know to what degree you can
> assume that.
There's the whole using DT bindings in ACPI bindings thing, but I have
little interest (or time) in supporting that. Maybe that's what's
happening here? I haven't looked. The whole concept is flawed IMO. It
may work for simple cases of key/value device properties, but the ACPI
model is quite different in how resources are described and managed.
Rob
There's the whole using DT bindings in ACPI bindings thing, but I have little interest (or time) in supporting that. Maybe that's what's happening here? I haven't looked. The whole concept is flawed IMO. It may work for simple cases of key/value device properties, but the ACPI model is quite different in how resources are described and managed.
Hi Rob,
I chatted with Khalil and he confirmed that the reason he added the device tree support was to follow the example of existing upstreamed i2c drivers (even if we have no support for it and all our customers have to use our firmware including UEFI ACPI tables). So it is unrelated to DT bindings in ACPI bindings. He agrees that it is better to just remove the device tree support (from the driver itself and from the documentation). So if you don't have any objections, I will remove it.
> i2c drivers (even if we have no support for it and all our customers
> have to use our firmware including UEFI ACPI tables). So it is
Because of the "custom firmware is needed and it is only ACPI" fact, I
think this is one of the rare cases where we can actually remove DT
support.