Re: [Openipmi-developer] [PATCH 33/34] drivers: remove incorrect of_match_ptr/ACPI_PTR annotations
On 03/04/2024 10:06, Arnd Bergmann wrote: > From: Arnd Bergmann > > When building with CONFIG_OF and/or CONFIG_ACPI disabled but W=1 extra > warnings enabled, a lot of driver cause a warning about an unused > ID table: > > drivers/char/tpm/tpm_ftpm_tee.c:356:34: error: unused variable > 'of_ftpm_tee_ids' [-Werror,-Wunused-const-variable] > drivers/dma/img-mdc-dma.c:863:34: error: unused variable 'mdc_dma_of_match' > [-Werror,-Wunused-const-variable] > drivers/fpga/versal-fpga.c:62:34: error: unused variable > 'versal_fpga_of_match' [-Werror,-Wunused-const-variable] > drivers/i2c/muxes/i2c-mux-ltc4306.c:200:34: error: unused variable > 'ltc4306_of_match' [-Werror,-Wunused-const-variable] > drivers/i2c/muxes/i2c-mux-reg.c:242:34: error: unused variable > 'i2c_mux_reg_of_match' [-Werror,-Wunused-const-variable] > drivers/memory/pl353-smc.c:62:34: error: unused variable > 'pl353_smc_supported_children' [-Werror,-Wunused-const-variable] > drivers/regulator/pbias-regulator.c:136:34: error: unused variable > 'pbias_of_match' [-Werror,-Wunused-const-variable] > drivers/regulator/twl-regulator.c:552:34: error: unused variable > 'twl_of_match' [-Werror,-Wunused-const-variable] > drivers/regulator/twl6030-regulator.c:645:34: error: unused variable > 'twl_of_match' [-Werror,-Wunused-const-variable] > drivers/scsi/hisi_sas/hisi_sas_v2_hw.c:3635:36: error: unused variable > 'sas_v2_acpi_match' [-Werror,-Wunused-const-variable] > drivers/staging/pi433/pi433_if.c:1359:34: error: unused variable > 'pi433_dt_ids' [-Werror,-Wunused-const-variable] > drivers/tty/serial/amba-pl011.c:2945:34: error: unused variable > 'sbsa_uart_of_match' [-Werror,-Wunused-const-variable] > > The fix is always to just remove the of_match_ptr() and ACPI_PTR() wrappers > that remove the reference, rather than adding another #ifdef just for build > testing for a configuration that doesn't matter in practice. > > I considered splitting up the large patch into per subsystem patches, but > since > it's really just the same thing everywhere it feels better to do it all at > once. > > Signed-off-by: Arnd Bergmann > --- > drivers/char/ipmi/ipmb_dev_int.c | 2 +- > drivers/char/tpm/tpm_ftpm_tee.c | 2 +- > drivers/dma/img-mdc-dma.c | 2 +- > drivers/fpga/versal-fpga.c| 2 +- > drivers/hid/hid-google-hammer.c | 6 ++ > drivers/i2c/muxes/i2c-mux-ltc4306.c | 2 +- > drivers/i2c/muxes/i2c-mux-reg.c | 2 +- > drivers/input/touchscreen/wdt87xx_i2c.c | 2 +- > drivers/mux/adg792a.c | 2 +- > drivers/net/ethernet/apm/xgene-v2/main.c | 2 +- > drivers/net/ethernet/hisilicon/hns_mdio.c | 2 +- > drivers/regulator/pbias-regulator.c | 2 +- > drivers/regulator/twl-regulator.c | 2 +- > drivers/regulator/twl6030-regulator.c | 2 +- I covered regulators here the same way: https://lore.kernel.org/all/20230310214553.275450-5-krzysztof.kozlow...@linaro.org/ but just like SPI and ASoC, Mark did not agree to pick them up. Best regards, Krzysztof ___ Openipmi-developer mailing list Openipmi-developer@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openipmi-developer
[Openipmi-developer] [PATCH] dt-bindings: ipmi: aspeed, ast2400-kcs-bmc: drop unneeded quotes
Cleanup bindings dropping unneeded quotes. Once all these are fixed, checking for this can be enabled in yamllint. Signed-off-by: Krzysztof Kozlowski --- .../devicetree/bindings/ipmi/aspeed,ast2400-kcs-bmc.yaml | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Documentation/devicetree/bindings/ipmi/aspeed,ast2400-kcs-bmc.yaml b/Documentation/devicetree/bindings/ipmi/aspeed,ast2400-kcs-bmc.yaml index 4ff6fabfcb30..129e32c4c774 100644 --- a/Documentation/devicetree/bindings/ipmi/aspeed,ast2400-kcs-bmc.yaml +++ b/Documentation/devicetree/bindings/ipmi/aspeed,ast2400-kcs-bmc.yaml @@ -41,7 +41,7 @@ properties: - description: STR register aspeed,lpc-io-reg: -$ref: '/schemas/types.yaml#/definitions/uint32-array' +$ref: /schemas/types.yaml#/definitions/uint32-array minItems: 1 maxItems: 2 description: | @@ -50,7 +50,7 @@ properties: status address may be optionally provided. aspeed,lpc-interrupts: -$ref: "/schemas/types.yaml#/definitions/uint32-array" +$ref: /schemas/types.yaml#/definitions/uint32-array minItems: 2 maxItems: 2 description: | @@ -63,12 +63,12 @@ properties: kcs_chan: deprecated: true -$ref: '/schemas/types.yaml#/definitions/uint32' +$ref: /schemas/types.yaml#/definitions/uint32 description: The LPC channel number in the controller kcs_addr: deprecated: true -$ref: '/schemas/types.yaml#/definitions/uint32' +$ref: /schemas/types.yaml#/definitions/uint32 description: The host CPU IO map address required: -- 2.34.1 ___ Openipmi-developer mailing list Openipmi-developer@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openipmi-developer
Re: [Openipmi-developer] [PATCH v3] dt-binding: ipmi: add fallback to npcm845 compatible
On 08/08/2022 15:26, Corey Minyard wrote:
> On Mon, Aug 08, 2022 at 11:11:16AM +0300, Krzysztof Kozlowski wrote:
>> On 08/08/2022 09:54, Tomer Maimon wrote:
>>> Add to npcm845 KCS compatible string a fallback to npcm750 KCS compatible
>>> string becuase NPCM845 and NPCM750 BMCs are using identical KCS modules.
>>>
>>> Fixes: 84261749e58a ("dt-bindings: ipmi: Add npcm845 compatible")
>>> Signed-off-by: Tomer Maimon
>>
>>
>> Acked-by: Krzysztof Kozlowski
>
> Ok, I think I understand how this is supposed to work. It's not
> altogether clear from the device tree documentation. It says in
> Documentation/devicetree/bindings/writing-bindings.rst:
>
> - DO make 'compatible' properties specific. DON'T use wildcards in compatible
> strings. DO use fallback compatibles when devices are the same as or a
> subset
> of prior implementations. DO add new compatibles in case there are new
> features or bugs.
This documentation is short, so it explains what should be done, not
exactly why it should be done. If we wanted "why" this would not be set
of 4 sentences but twice more...
>
> AFAICT, there are no new features or bugs, just a new SOC with the same
> device. In general usage I have seen, you would just use the same
> compatible.
Most of the usages are like shown here. There are several other cases.
It's the same with poor or good code - you will always find both patterns.
> However, if I understand this, that last sentence should say:
>
> DO add new compatibles in case there is a new version of hardware with
> the possibility of new features and/or bugs.
>
> Also, the term "specific" is, ironically, vague. Specific to what?
To me it is rather clear. Specific as in first meanings of the word (1,
3, 4 and 5):
https://en.wiktionary.org/wiki/specific
nuvoton,npcm7xx-kcs-bmc would not be definite (allows more meanings),
unique (in terms of devices it expresses), distinctive (as two different
devices use the same) or serving to identify one thing (again - two SoCs).
What other meaning do you think of?
>
> It would be nice to have something added to "Typical cases and caveats"
> that says:
>
> - If you are writing a binding for a new device that is the same as, or
> a superset of another existing device, add a new specific compatible
> for the new device followed by a compatible for the existing device.
> That way, if the device has new bugs or new specific features are
> added, you can add workarounds without modifying the device tree.
>
> Anyway, I have added this to my tree with your ack.
Fantastic, thanks!
Best regards,
Krzysztof
___
Openipmi-developer mailing list
Openipmi-developer@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openipmi-developer
Re: [Openipmi-developer] [PATCH v3] dt-binding: ipmi: add fallback to npcm845 compatible
On 08/08/2022 09:54, Tomer Maimon wrote:
> Add to npcm845 KCS compatible string a fallback to npcm750 KCS compatible
> string becuase NPCM845 and NPCM750 BMCs are using identical KCS modules.
>
> Fixes: 84261749e58a ("dt-bindings: ipmi: Add npcm845 compatible")
> Signed-off-by: Tomer Maimon
Acked-by: Krzysztof Kozlowski
Best regards,
Krzysztof
___
Openipmi-developer mailing list
Openipmi-developer@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openipmi-developer
Re: [Openipmi-developer] [PATCH v2] dt-binding: ipmi: add fallback to npcm845 compatible
On 07/08/2022 09:51, Tomer Maimon wrote:
> Hi Krzysztof,
>
> Thanks for your review.
>
> On Fri, 5 Aug 2022 at 09:36, Krzysztof Kozlowski
> wrote:
>>
>> On 04/08/2022 20:18, Tomer Maimon wrote:
>>> Add to npcm845 KCS compatible string a fallback to npcm750 KCS compatible
>>> string becuase NPCM845 and NPCM750 BMCs are using identical KCS modules.
>>>
>>> Signed-off-by: Tomer Maimon
>>
>> Your previous commit adding that compatible was simply wrong and not
>> matching the driver and it is not the first time. I think all Nuvoton
>> patches need much more careful review :(
> Will do and sorry about all the mess...
>>
>> You forgot the fixes tag:
>>
>> Fixes: 84261749e58a ("dt-bindings: ipmi: Add npcm845 compatible")
> Will add the tag next version.
You received a bit different review from Corey, so to be clear:
1. Your approach is correct, assuming the devices are really compatible.
2. Add a fixes tag and send a v3, to get my ack.
Best regards,
Krzysztof
___
Openipmi-developer mailing list
Openipmi-developer@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openipmi-developer
Re: [Openipmi-developer] [PATCH v2] dt-binding: ipmi: add fallback to npcm845 compatible
On 07/08/2022 18:05, Corey Minyard wrote: >> We had a disscation with Arnd, Arnd asked us to use a fallback as we >> did here if NPCM8XX device module is similar to NPCM7XX module: >> https://lore.kernel.org/lkml/20220522155046.260146-5-tmaimo...@gmail.com/ >> >> I think we should use a fallback to describe the NPCM8XX KCS in the >> dt-binding document. > > I'm ok with that option. I guess I should have mentioned it. Add > nuvoton,npcm-kcs-bmc to the driver's of_device_id table. To be clear - NAK. It's not a specific compatible, therefore it is not appropriate at all. > Then use that > in that compatible string in the device tree. Leave the 750 compatible > string in the table for backwards compatibility. > > There's no point in having a bunch of those strings if they are all the > same. If a new one comes out that is different, we can handle that when > the time comes.\ There is a point. It's exactly how we do in all DT bindings. It is recommended by writing bindings document. Best regards, Krzysztof ___ Openipmi-developer mailing list Openipmi-developer@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openipmi-developer
Re: [Openipmi-developer] [PATCH v2] dt-binding: ipmi: add fallback to npcm845 compatible
On 07/08/2022 16:54, Tomer Maimon wrote: > On Sun, 7 Aug 2022 at 15:11, Corey Minyard wrote: >> >> On Sun, Aug 07, 2022 at 11:03:56AM +0300, Tomer Maimon wrote: >>> Hi Corey, >>> >>> Thanks for your comment. >>> >>> On Fri, 5 Aug 2022 at 14:58, Corey Minyard wrote: On Thu, Aug 04, 2022 at 09:18:00PM +0300, Tomer Maimon wrote: > Add to npcm845 KCS compatible string a fallback to npcm750 KCS compatible > string becuase NPCM845 and NPCM750 BMCs are using identical KCS modules. > > Signed-off-by: Tomer Maimon > --- > Documentation/devicetree/bindings/ipmi/npcm7xx-kcs-bmc.txt | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/ipmi/npcm7xx-kcs-bmc.txt > b/Documentation/devicetree/bindings/ipmi/npcm7xx-kcs-bmc.txt > index cbc10a68ddef..4fda76e63396 100644 > --- a/Documentation/devicetree/bindings/ipmi/npcm7xx-kcs-bmc.txt > +++ b/Documentation/devicetree/bindings/ipmi/npcm7xx-kcs-bmc.txt > @@ -7,7 +7,7 @@ used to perform in-band IPMI communication with their > host. > Required properties: > - compatible : should be one of > "nuvoton,npcm750-kcs-bmc" > -"nuvoton,npcm845-kcs-bmc" > +"nuvoton,npcm845-kcs-bmc", "nuvoton,npcm750-kcs-bmc" This is just wrong. The compatible is supposed to identify the device, not the board the device is on. I think compatible here should be "npcm7xx-kcs-bmc", and just use that everywhere. It's fine if that is used on a board named npcm845. >>> The NPCM8XX is not a board, The Nuvoton NPCM8XX is a fourth-generation >>> BMC SoC device family. >> >> Ok, but same principle applies. >> >> If the device is exactly the same, then you would only use one of the >> "npcm7xx-kcs-bmc" and put that in both device trees. You can use >> "nuvoton,npcm750-kcs-bmc", it's really not that important. Or even >> "nuvoton,npcm-kcs-bmc" > If we use "nuvoton, npcm-kcs-bmc" we should take care of backward dts > compatibility, and I am not sure we like to change NPCM KCS driver. >> >> If the device has a minor difference that can be expressed in a >> parameter, then create a parameter for it. >> >> If the device has enough differences that a parameter or two doesn't >> cover it, then you put either nuvoton,npcm750-kcs-bmc or >> nuvoton,npcm750-kcs-bmc in the device tree. Not both. Then you need >> two entries in the of_device_id array and you use the data field or >> something to express the difference. >> >> Since there appears to be no difference, just put >> "nuvoton,npcm750-kcs-bmc" in the npcm845 and I will drop the patch >> adding all this. Then a patch can be added saying it applies to both >> the 7xx and 8xx series of BMC SOCs. If you want to change the name, >> then a patch will be needed for that, but then you will need multiple >> entries in your device tree, but you would not document it as such, as >> there would only be one that applies for this kernel. > > It little bit confusing to use nuvoton,npcm750-kcs-bmc that are > related to NPCM7XX for NPCM8XX KCS. > We can use the generic name "nuvoton, npcm-kcs-bmc" as you suggested No, please don't. It will be NAKed. :) > above but we should take care of backward dts compatibility, and I am > not sure we like to change NPCM KCS driver. > > We had a disscation with Arnd, Arnd asked us to use a fallback as we > did here if NPCM8XX device module is similar to NPCM7XX module: > https://lore.kernel.org/lkml/20220522155046.260146-5-tmaimo...@gmail.com/ > > I think we should use a fallback to describe the NPCM8XX KCS in the > dt-binding document. Best regards, Krzysztof ___ Openipmi-developer mailing list Openipmi-developer@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openipmi-developer
Re: [Openipmi-developer] [PATCH v2] dt-binding: ipmi: add fallback to npcm845 compatible
On 07/08/2022 14:11, Corey Minyard wrote: > On Sun, Aug 07, 2022 at 11:03:56AM +0300, Tomer Maimon wrote: >> Hi Corey, >> >> Thanks for your comment. >> >> On Fri, 5 Aug 2022 at 14:58, Corey Minyard wrote: >>> >>> On Thu, Aug 04, 2022 at 09:18:00PM +0300, Tomer Maimon wrote: Add to npcm845 KCS compatible string a fallback to npcm750 KCS compatible string becuase NPCM845 and NPCM750 BMCs are using identical KCS modules. Signed-off-by: Tomer Maimon --- Documentation/devicetree/bindings/ipmi/npcm7xx-kcs-bmc.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/ipmi/npcm7xx-kcs-bmc.txt b/Documentation/devicetree/bindings/ipmi/npcm7xx-kcs-bmc.txt index cbc10a68ddef..4fda76e63396 100644 --- a/Documentation/devicetree/bindings/ipmi/npcm7xx-kcs-bmc.txt +++ b/Documentation/devicetree/bindings/ipmi/npcm7xx-kcs-bmc.txt @@ -7,7 +7,7 @@ used to perform in-band IPMI communication with their host. Required properties: - compatible : should be one of "nuvoton,npcm750-kcs-bmc" -"nuvoton,npcm845-kcs-bmc" +"nuvoton,npcm845-kcs-bmc", "nuvoton,npcm750-kcs-bmc" >>> >>> This is just wrong. The compatible is supposed to identify the device, >>> not the board the device is on. I think compatible here should be >>> "npcm7xx-kcs-bmc", and just use that everywhere. It's fine if that is >>> used on a board named npcm845. >> The NPCM8XX is not a board, The Nuvoton NPCM8XX is a fourth-generation >> BMC SoC device family. > > Ok, but same principle applies. > > If the device is exactly the same, then you would only use one of the > "npcm7xx-kcs-bmc" and put that in both device trees. You can use > "nuvoton,npcm750-kcs-bmc", it's really not that important. Or even > "nuvoton,npcm-kcs-bmc" No, because it is too generic. Compatibles must be specific. > > If the device has a minor difference that can be expressed in a > parameter, then create a parameter for it. > > If the device has enough differences that a parameter or two doesn't > cover it, then you put either nuvoton,npcm750-kcs-bmc or > nuvoton,npcm750-kcs-bmc in the device tree. Not both. Then you need > two entries in the of_device_id array and you use the data field or > something to express the difference. It's quite common to have generic and specific compatibles for compatible devices and a driver which can match to both of them. I don't understand where is exactly the problem here? > > Since there appears to be no difference, just put > "nuvoton,npcm750-kcs-bmc" in the npcm845 and I will drop the patch > adding all this. T Again no, because recommended (also writing bindings document) is always to have a specific compatible. > hen a patch can be added saying it applies to both > the 7xx and 8xx series of BMC SOCs. If you want to change the name, > then a patch will be needed for that, but then you will need multiple > entries in your device tree, but you would not document it as such, as > there would only be one that applies for this kernel. > > I'm pretty sure the only reason to have muliple compatible entries in a > device tree is to cover multiple kernels where the name changed. Best regards, Krzysztof ___ Openipmi-developer mailing list Openipmi-developer@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openipmi-developer
Re: [Openipmi-developer] [PATCH v2] dt-binding: ipmi: add fallback to npcm845 compatible
On 05/08/2022 13:58, Corey Minyard wrote: > On Thu, Aug 04, 2022 at 09:18:00PM +0300, Tomer Maimon wrote: >> Add to npcm845 KCS compatible string a fallback to npcm750 KCS compatible >> string becuase NPCM845 and NPCM750 BMCs are using identical KCS modules. >> >> Signed-off-by: Tomer Maimon >> --- >> Documentation/devicetree/bindings/ipmi/npcm7xx-kcs-bmc.txt | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/Documentation/devicetree/bindings/ipmi/npcm7xx-kcs-bmc.txt >> b/Documentation/devicetree/bindings/ipmi/npcm7xx-kcs-bmc.txt >> index cbc10a68ddef..4fda76e63396 100644 >> --- a/Documentation/devicetree/bindings/ipmi/npcm7xx-kcs-bmc.txt >> +++ b/Documentation/devicetree/bindings/ipmi/npcm7xx-kcs-bmc.txt >> @@ -7,7 +7,7 @@ used to perform in-band IPMI communication with their host. >> Required properties: >> - compatible : should be one of >> "nuvoton,npcm750-kcs-bmc" >> -"nuvoton,npcm845-kcs-bmc" >> +"nuvoton,npcm845-kcs-bmc", "nuvoton,npcm750-kcs-bmc" > > This is just wrong. The compatible is supposed to identify the device, > not the board the device is on. I think compatible here should be > "npcm7xx-kcs-bmc", and just use that everywhere. It's fine if that is No, because you propose to use wildcards as compatible which is not correct. Compatibles must be specific. Best regards, Krzysztof ___ Openipmi-developer mailing list Openipmi-developer@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openipmi-developer
Re: [Openipmi-developer] [PATCH v2] dt-binding: ipmi: add fallback to npcm845 compatible
On 04/08/2022 20:18, Tomer Maimon wrote:
> Add to npcm845 KCS compatible string a fallback to npcm750 KCS compatible
> string becuase NPCM845 and NPCM750 BMCs are using identical KCS modules.
>
> Signed-off-by: Tomer Maimon
Your previous commit adding that compatible was simply wrong and not
matching the driver and it is not the first time. I think all Nuvoton
patches need much more careful review :(
You forgot the fixes tag:
Fixes: 84261749e58a ("dt-bindings: ipmi: Add npcm845 compatible")
Best regards,
Krzysztof
___
Openipmi-developer mailing list
Openipmi-developer@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openipmi-developer
Re: [Openipmi-developer] [PATCH v1] dt-binding: ipmi: add fallback to npcm845 compatible
On 04/08/2022 16:55, Tomer Maimon wrote: > Add to npcm845 KCS compatible string a fallback to npcm750 KCS compatible > string. It's easily visible in the code what you are doing, so instead you must explain why you are doing it. Best regards, Krzysztof ___ Openipmi-developer mailing list Openipmi-developer@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openipmi-developer
Re: [Openipmi-developer] [PATCH v1 1/2] dt-bindings: ipmi: Add npcm845 compatible
On 17/07/2022 14:11, Tomer Maimon wrote: > Add a compatible string for Nuvoton BMC NPCM845 KCS and modify NPCM KCS > description to support all NPCM BMC SoC. > > Signed-off-by: Tomer Maimon Acked-by: Krzysztof Kozlowski Best regards, Krzysztof ___ Openipmi-developer mailing list Openipmi-developer@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openipmi-developer
Re: [Openipmi-developer] [PATCH v7 2/3] bindings: ipmi: Add binding for SSIF BMC driver
On 22/04/2022 06:16, Quan Nguyen wrote: > Added Krzysztof Kozlowski > as I'm not aware of the email change The email change from @canonical.com was 1.5 months ago, so it would be better if rebase your changes on more recent Linux kernel. You get all proper .mailmap entries. Best regards, Krzysztof ___ Openipmi-developer mailing list Openipmi-developer@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openipmi-developer
Re: [Openipmi-developer] [PATCH] HPE BMC GXP SUPPORT
On 03/02/2022 18:07, Verdun, Jean-Marie wrote: > >> Maybe it does not look like, but this is actually a v2. Nick was asked >> to change the naming for the nodes already in v1. Unfortunately it did >> not happen, so we have vuart, spifi, vic and more. > >> It is a waste of reviewers' time to ask them to perform the same review >> twice or to ignore their comments. > > Hi Krysztof, > > Accept our apologies if you think you lose your time, it is clearly not > our > intent. > > This is the first time that we (I mean the team) introduce a new arch into > the linux kernel and I must admit that we had hard time to understand > from which angle we needed to start. > > I will probably write a Documentation afterward, as it is easy to find doc > on how to introduce a patch or a driver, but not when you want to > introduce a new chip. > > We are trying to do our best, and clearly want to follow all of your > inputs. > Mistakes happen and they are clearly not intentional and not driven in > a way to make you lose your time. > > Helping others, and teaching something new is definitely a way to > optimize your time and this is what you are currently doing with us. > > We appreciate it and hope you will too. I understand, I also maybe over-reacted on this. Just please go through the comments you got for first submission and either apply them or respond why you disagree. The next submissions (patchset split into several commits) should be a v3, preferably with cover letter (git format-patch --cover-letter -v3 ...) where you can document also changes you did to the patchset. It looks for example like this: https://lore.kernel.org/linux-samsung-soc/31da451b-a36c-74fb-5667-d4193284c...@canonical.com/T/#mf98d2ac27a8481dc69dd110f9861c8318cade252 or like this (where changelogs are in each patch, although ordering is not correct because dt-bindings should be the first in the series): https://lore.kernel.org/all/20220103233948.198119-1-mr.bossman...@gmail.com/ Best regards, Krzysztof ___ Openipmi-developer mailing list Openipmi-developer@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openipmi-developer
Re: [Openipmi-developer] [PATCH] HPE BMC GXP SUPPORT
On 02/02/2022 17:52, nick.hawk...@hpe.com wrote: > From: Nick Hawkins > > GXP is the name of the HPE SoC. > This SoC is used to implement BMC features of HPE servers > (all ProLiant, Synergy, and many Apollo, and Superdome machines) > It does support many features including: > ARMv7 architecture, and it is based on a Cortex A9 core > Use an AXI bus to which > a memory controller is attached, as well as > multiple SPI interfaces to connect boot flash, > and ROM flash, a 10/100/1000 Mac engine which > supports SGMII (2 ports) and RMII > Multiple I2C engines to drive connectivity with a host > infrastructure > A video engine which support VGA and DP, as well as > an hardware video encoder > Multiple PCIe ports > A PECI interface, and LPC eSPI > Multiple UART for debug purpose, and Virtual UART for host > connectivity > A GPIO engine > This Patch Includes: > Documentation for device tree bindings > Device Tree Bindings > GXP Timer Support > GXP Architecture Support > 1. Please version your patchses and document the changes under ---. 2. With your v1 I responded what has to be separate patch. This was totally ignored here, so no. You have to follow this. 3. Please run checkpatch and be sure there are no warnings. 4. Bindings in dtschema, not in text. Best regards, Krzysztof Best regards, Krzysztof ___ Openipmi-developer mailing list Openipmi-developer@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openipmi-developer
Re: [Openipmi-developer] [PATCH] HPE BMC GXP SUPPORT
On 03/02/2022 15:29, Rob Herring wrote:
> On Wed, Feb 2, 2022 at 10:55 AM wrote:
>>
>> From: Nick Hawkins
>>
(...)
>> +
>> + vuart_a: vuart_a@80fd0200 {
>
> serial@...
Maybe it does not look like, but this is actually a v2. Nick was asked
to change the naming for the nodes already in v1. Unfortunately it did
not happen, so we have vuart, spifi, vic and more.
It is a waste of reviewers' time to ask them to perform the same review
twice or to ignore their comments.
>
>> + compatible = "hpe,gxp-vuart";
>> + reg = <0x80fd0200 0x100>;
>> + interrupts = <2>;
>> + interrupt-parent = <&vic1>;
>> + clock-frequency = <1846153>;
>> + reg-shift = <0>;
>> + status = "okay";
>> + serial-line = <3>;
>> + vuart_cfg = <&vuart_a_cfg>;
>> + };
(...)
>> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml
>> b/Documentation/devicetree/bindings/vendor-prefixes.yaml
>> index 294093d45a23..913f722a6b8d 100644
>> --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
>> +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
>> @@ -514,7 +514,9 @@ patternProperties:
>>"^hoperun,.*":
>> description: Jiangsu HopeRun Software Co., Ltd.
>>"^hp,.*":
>> -description: Hewlett Packard
>> +description: Hewlett Packard Inc.
>
> Why are you changing this one?
I guess this is squashing of my patch:
https://lore.kernel.org/all/20220127075229.10299-1-krzysztof.kozlow...@canonical.com/
which is fine to me, but vendor changve should be a separate commit with
its own explanation. Now it looks indeed weird.
>
>> + "^hpe,.*":
>
> You used HPE elsewhere... Lowercase is preferred.
Best regards,
Krzysztof
___
Openipmi-developer mailing list
Openipmi-developer@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openipmi-developer
[Openipmi-developer] [RESEND PATCH] char: ipmi: Drop owner assignment from i2c_driver
i2c_driver does not need to set an owner because i2c_register_driver()
will set it.
Signed-off-by: Krzysztof Kozlowski
---
The coccinelle script which generated the patch was sent here:
http://www.spinics.net/lists/kernel/msg2029903.html
---
drivers/char/ipmi/ipmi_ssif.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/char/ipmi/ipmi_ssif.c b/drivers/char/ipmi/ipmi_ssif.c
index 90e624662257..5f1c3d08ba65 100644
--- a/drivers/char/ipmi/ipmi_ssif.c
+++ b/drivers/char/ipmi/ipmi_ssif.c
@@ -1959,7 +1959,6 @@ MODULE_DEVICE_TABLE(i2c, ssif_id);
static struct i2c_driver ssif_i2c_driver = {
.class = I2C_CLASS_HWMON,
.driver = {
- .owner = THIS_MODULE,
.name = DEVICE_NAME
},
.probe = ssif_probe,
--
1.9.1
--
___
Openipmi-developer mailing list
Openipmi-developer@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openipmi-developer
[Openipmi-developer] [PATCH] char: Drop owner assignment from i2c_driver
i2c_driver does not need to set an owner because i2c_register_driver()
will set it.
Signed-off-by: Krzysztof Kozlowski
---
The coccinelle script which generated the patch was sent here:
http://www.spinics.net/lists/kernel/msg2029903.html
---
drivers/char/ipmi/ipmi_ssif.c | 1 -
drivers/char/tpm/st33zp24/i2c.c | 1 -
drivers/char/tpm/tpm_i2c_atmel.c| 1 -
drivers/char/tpm/tpm_i2c_infineon.c | 1 -
drivers/char/tpm/tpm_i2c_nuvoton.c | 1 -
5 files changed, 5 deletions(-)
diff --git a/drivers/char/ipmi/ipmi_ssif.c b/drivers/char/ipmi/ipmi_ssif.c
index b043d8d45823..003a4ab3b660 100644
--- a/drivers/char/ipmi/ipmi_ssif.c
+++ b/drivers/char/ipmi/ipmi_ssif.c
@@ -1950,7 +1950,6 @@ MODULE_DEVICE_TABLE(i2c, ssif_id);
static struct i2c_driver ssif_i2c_driver = {
.class = I2C_CLASS_HWMON,
.driver = {
- .owner = THIS_MODULE,
.name = DEVICE_NAME
},
.probe = ssif_probe,
diff --git a/drivers/char/tpm/st33zp24/i2c.c b/drivers/char/tpm/st33zp24/i2c.c
index ad1ee180e0c2..309d2767c6a1 100644
--- a/drivers/char/tpm/st33zp24/i2c.c
+++ b/drivers/char/tpm/st33zp24/i2c.c
@@ -258,7 +258,6 @@ static SIMPLE_DEV_PM_OPS(st33zp24_i2c_ops,
st33zp24_pm_suspend,
static struct i2c_driver st33zp24_i2c_driver = {
.driver = {
- .owner = THIS_MODULE,
.name = TPM_ST33_I2C,
.pm = &st33zp24_i2c_ops,
.of_match_table = of_match_ptr(of_st33zp24_i2c_match),
diff --git a/drivers/char/tpm/tpm_i2c_atmel.c b/drivers/char/tpm/tpm_i2c_atmel.c
index 7a0ca78ad3c6..8dfb88b9739c 100644
--- a/drivers/char/tpm/tpm_i2c_atmel.c
+++ b/drivers/char/tpm/tpm_i2c_atmel.c
@@ -217,7 +217,6 @@ static struct i2c_driver i2c_atmel_driver = {
.remove = i2c_atmel_remove,
.driver = {
.name = I2C_DRIVER_NAME,
- .owner = THIS_MODULE,
.pm = &i2c_atmel_pm_ops,
.of_match_table = of_match_ptr(i2c_atmel_of_match),
},
diff --git a/drivers/char/tpm/tpm_i2c_infineon.c
b/drivers/char/tpm/tpm_i2c_infineon.c
index 33c5f360ab01..63d5d22e9e60 100644
--- a/drivers/char/tpm/tpm_i2c_infineon.c
+++ b/drivers/char/tpm/tpm_i2c_infineon.c
@@ -711,7 +711,6 @@ static struct i2c_driver tpm_tis_i2c_driver = {
.remove = tpm_tis_i2c_remove,
.driver = {
.name = "tpm_i2c_infineon",
- .owner = THIS_MODULE,
.pm = &tpm_tis_i2c_ops,
.of_match_table = of_match_ptr(tpm_tis_i2c_of_match),
},
diff --git a/drivers/char/tpm/tpm_i2c_nuvoton.c
b/drivers/char/tpm/tpm_i2c_nuvoton.c
index 9d42b7d78e50..847f1597fe9b 100644
--- a/drivers/char/tpm/tpm_i2c_nuvoton.c
+++ b/drivers/char/tpm/tpm_i2c_nuvoton.c
@@ -641,7 +641,6 @@ static struct i2c_driver i2c_nuvoton_driver = {
.remove = i2c_nuvoton_remove,
.driver = {
.name = I2C_DRIVER_NAME,
- .owner = THIS_MODULE,
.pm = &i2c_nuvoton_pm_ops,
.of_match_table = of_match_ptr(i2c_nuvoton_of_match),
},
--
1.9.1
--
Don't Limit Your Business. Reach for the Cloud.
GigeNET's Cloud Solutions provide you with the tools and support that
you need to offload your IT needs and focus on growing your business.
Configured For All Businesses. Start Your Cloud Today.
https://www.gigenetcloud.com/
___
Openipmi-developer mailing list
Openipmi-developer@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openipmi-developer
[Openipmi-developer] [PATCH] Drop owner assignment from i2c_driver (and platform left-overs)
Hi, The i2c drivers also do not have to set 'owner' field because i2c_register_driver() will do it instead. 'owner' is removed from i2c drivers, which I was able to compile with allyesconfig (arm, arm64, i386, x86_64, ppc64). Only compile-tested. The coccinelle script which generated the patch was sent here: http://www.spinics.net/lists/kernel/msg2029903.html Best regards, Krzysztof Krzysztof Kozlowski (1): char: Drop owner assignment from i2c_driver drivers/char/ipmi/ipmi_ssif.c | 1 - drivers/char/tpm/st33zp24/i2c.c | 1 - drivers/char/tpm/tpm_i2c_atmel.c| 1 - drivers/char/tpm/tpm_i2c_infineon.c | 1 - drivers/char/tpm/tpm_i2c_nuvoton.c | 1 - 5 files changed, 5 deletions(-) -- 1.9.1 -- Don't Limit Your Business. Reach for the Cloud. GigeNET's Cloud Solutions provide you with the tools and support that you need to offload your IT needs and focus on growing your business. Configured For All Businesses. Start Your Cloud Today. https://www.gigenetcloud.com/ ___ Openipmi-developer mailing list Openipmi-developer@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openipmi-developer
