| Message ID | 20240413071621.12509-3-michael.haener@siemens.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add ST33KTPM2XI2C chip to the TPM TIS I2C driver | expand |
On 13/04/2024 09:15, M. Haener wrote: > From: Michael Haener <michael.haener@siemens.com> > Please use subject prefixes matching the subsystem. You can get them for example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory your patch is touching. A nit, subject: drop second/last, redundant "binding". The "dt-bindings" prefix is already stating that these are bindings. See also: https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18 > Documentation/devicetree/bindings/tpm/tcg,tpm-tis-i2c.yaml | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/Documentation/devicetree/bindings/tpm/tcg,tpm-tis-i2c.yaml b/Documentation/devicetree/bindings/tpm/tcg,tpm-tis-i2c.yaml > index 3ab4434b7352..af7720dc4a12 100644 > --- a/Documentation/devicetree/bindings/tpm/tcg,tpm-tis-i2c.yaml > +++ b/Documentation/devicetree/bindings/tpm/tcg,tpm-tis-i2c.yaml > @@ -32,6 +32,7 @@ properties: > - enum: > - infineon,slb9673 > - nuvoton,npct75x > + - st,st33ktpm2xi2c I got only one patch, but if these are compatible, why do you need second patch? Plus binding come before users. Please explain what are you doing in other patch. Best regards, Krzysztof
On 13/04/2024 09:15, M. Haener wrote: > From: Michael Haener <michael.haener@siemens.com> > > Add the ST chip st33ktpm2xi2c to the supported compatible strings of the > TPM TIS I2C schema. The Chip is compliant with the TCG PC Client TPM > Profile specification. > > For reference, a datasheet is available at: > https://www.st.com/resource/en/data_brief/st33ktpm2xi2c.pdf > > Reviewed-by: Alexander Sverdlin <alexander.sverdlin@siemens.com> > Signed-off-by: Michael Haener <michael.haener@siemens.com> > --- Not tested... Please use scripts/get_maintainers.pl to get a list of necessary people and lists to CC. It might happen, that command when run on an older kernel, gives you outdated entries. Therefore please be sure you base your patches on recent Linux kernel. Tools like b4 or scripts/get_maintainer.pl provide you proper list of people, so fix your workflow. Tools might also fail if you work on some ancient tree (don't, instead use mainline), work on fork of kernel (don't, instead use mainline) or you ignore some maintainers (really don't). Just use b4 and everything should be fine, although remember about `b4 prep --auto-to-cc` if you added new patches to the patchset. You missed at least devicetree list (maybe more), so this won't be tested by automated tooling. Please kindly resend and include all necessary To/Cc entries. Best regards, Krzysztof
On Sat, Apr 13, 2024 at 10:10:49AM +0200, Krzysztof Kozlowski wrote: > Please use subject prefixes matching the subsystem. You can get them for > example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory > your patch is touching. To be fair, "dt-bindings: tpm: " is actually the only prefix used so far for the file that's touched here: $ git log --oneline Documentation/devicetree/bindings/tpm/tcg,tpm-tis-i2c.yaml 26c9d15 dt-bindings: tpm: Consolidate TCG TIS bindings Personally I don't think we need to differentiate between spi/i2c/mmio bindings in the prefix, so the prefix used by Michael seems fine. > A nit, subject: drop second/last, redundant "binding". The "dt-bindings" > prefix is already stating that these are bindings. > See also: > https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18 Right, so maybe just: dt-bindings: tpm: Add st,st33ktpm2xi2c ? > I got only one patch, but if these are compatible, why do you need > second patch? Plus binding come before users. Right, the order of the patches needs to be reversed it seems. Thanks, Lukas
On 13/04/2024 12:18, Lukas Wunner wrote: > On Sat, Apr 13, 2024 at 10:10:49AM +0200, Krzysztof Kozlowski wrote: >> Please use subject prefixes matching the subsystem. You can get them for >> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory >> your patch is touching. > > To be fair, "dt-bindings: tpm: " is actually the only prefix used > so far for the file that's touched here: > > $ git log --oneline Documentation/devicetree/bindings/tpm/tcg,tpm-tis-i2c.yaml > 26c9d15 dt-bindings: tpm: Consolidate TCG TIS bindings Command should be run on the directory, but anyway you are right. So it's fine. > > Personally I don't think we need to differentiate between spi/i2c/mmio > bindings in the prefix, so the prefix used by Michael seems fine. > > >> A nit, subject: drop second/last, redundant "binding". The "dt-bindings" >> prefix is already stating that these are bindings. >> See also: >> https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18 > > Right, so maybe just: > > dt-bindings: tpm: Add st,st33ktpm2xi2c > > ? > > >> I got only one patch, but if these are compatible, why do you need >> second patch? Plus binding come before users. > > Right, the order of the patches needs to be reversed it seems. What is the second patch? Device is or is not compatible? Best regards, Krzysztof
On Sat, Apr 13, 2024 at 12:23:47PM +0200, Krzysztof Kozlowski wrote: > On 13/04/2024 12:18, Lukas Wunner wrote: > > On Sat, Apr 13, 2024 at 10:10:49AM +0200, Krzysztof Kozlowski wrote: > > > I got only one patch, but if these are compatible, why do you need > > > second patch? Plus binding come before users. > > > > Right, the order of the patches needs to be reversed it seems. > > What is the second patch? Device is or is not compatible? The other patch just adds an entry to of_tis_i2c_match[] in the driver, pretty unspectacular: https://lore.kernel.org/all/20240413071621.12509-2-michael.haener@siemens.com/ Unfortunately it was only cc'ed to TPM driver maintainers.
On 13/04/2024 12:34, Lukas Wunner wrote: > On Sat, Apr 13, 2024 at 12:23:47PM +0200, Krzysztof Kozlowski wrote: >> On 13/04/2024 12:18, Lukas Wunner wrote: >>> On Sat, Apr 13, 2024 at 10:10:49AM +0200, Krzysztof Kozlowski wrote: >>>> I got only one patch, but if these are compatible, why do you need >>>> second patch? Plus binding come before users. >>> >>> Right, the order of the patches needs to be reversed it seems. >> >> What is the second patch? Device is or is not compatible? > > The other patch just adds an entry to of_tis_i2c_match[] in the driver, > pretty unspectacular: > > https://lore.kernel.org/all/20240413071621.12509-2-michael.haener@siemens.com/ > Then why is it needed? To re-iterate: "Device is or is not compatible?" Decide, one of the two patches is wrong. Best regards, Krzysztof
On Sat, Apr 13, 2024 at 12:43:38PM +0200, Krzysztof Kozlowski wrote: > On 13/04/2024 12:34, Lukas Wunner wrote: > > The other patch just adds an entry to of_tis_i2c_match[] in the driver, > > pretty unspectacular: > > > > https://lore.kernel.org/all/20240413071621.12509-2-michael.haener@siemens.com/ > > Then why is it needed? The binding requires two entries in the compatible string used in the DT, the chip name followed by the generic string: items: - enum: - infineon,slb9673 - nuvoton,npct75x - const: tcg,tpm-tis-i2c This allows us to deal with device-specific quirks, should they pop up (e.g. special timing requirements, hardware bugs). We don't know in advance if they will be discovered, but if they are, it's cumbersome to determine after the fact which products (and thus DTs) are affected. So having the name of the actual chip used on the board has value. Thanks, Lukas
On 13/04/2024 12:51, Lukas Wunner wrote: > On Sat, Apr 13, 2024 at 12:43:38PM +0200, Krzysztof Kozlowski wrote: >> On 13/04/2024 12:34, Lukas Wunner wrote: >>> The other patch just adds an entry to of_tis_i2c_match[] in the driver, >>> pretty unspectacular: >>> >>> https://lore.kernel.org/all/20240413071621.12509-2-michael.haener@siemens.com/ >> >> Then why is it needed? > > The binding requires two entries in the compatible string used in the DT, > the chip name followed by the generic string: > > items: > - enum: > - infineon,slb9673 > - nuvoton,npct75x > - const: tcg,tpm-tis-i2c > > This allows us to deal with device-specific quirks, should they pop up > (e.g. special timing requirements, hardware bugs). We don't know in > advance if they will be discovered, but if they are, it's cumbersome > to determine after the fact which products (and thus DTs) are affected. > So having the name of the actual chip used on the board has value. So you say devices are compatible. Then the second patch is wrong. I cannot respond to it, though... so NAK-here-for-second-patch. Best regards, Krzysztof
On Sat, Apr 13, 2024 at 12:53:25PM +0200, Krzysztof Kozlowski wrote: > On 13/04/2024 12:51, Lukas Wunner wrote: > > The binding requires two entries in the compatible string used in the DT, > > the chip name followed by the generic string: > > > > items: > > - enum: > > - infineon,slb9673 > > - nuvoton,npct75x > > - const: tcg,tpm-tis-i2c > > > > This allows us to deal with device-specific quirks, should they pop up > > (e.g. special timing requirements, hardware bugs). We don't know in > > advance if they will be discovered, but if they are, it's cumbersome > > to determine after the fact which products (and thus DTs) are affected. > > So having the name of the actual chip used on the board has value. > > So you say devices are compatible. Then the second patch is wrong. > > I cannot respond to it, though... so NAK-here-for-second-patch. I disagree. It's ugly to have inconsistencies between the DT bindings and the driver. So I think patch [1/2] in this series is fine. Thanks, Lukas
On 13/04/2024 12:56, Lukas Wunner wrote: > On Sat, Apr 13, 2024 at 12:53:25PM +0200, Krzysztof Kozlowski wrote: >> On 13/04/2024 12:51, Lukas Wunner wrote: >>> The binding requires two entries in the compatible string used in the DT, >>> the chip name followed by the generic string: >>> >>> items: >>> - enum: >>> - infineon,slb9673 >>> - nuvoton,npct75x >>> - const: tcg,tpm-tis-i2c >>> >>> This allows us to deal with device-specific quirks, should they pop up >>> (e.g. special timing requirements, hardware bugs). We don't know in >>> advance if they will be discovered, but if they are, it's cumbersome >>> to determine after the fact which products (and thus DTs) are affected. >>> So having the name of the actual chip used on the board has value. >> >> So you say devices are compatible. Then the second patch is wrong. >> >> I cannot respond to it, though... so NAK-here-for-second-patch. > > I disagree. It's ugly to have inconsistencies between the DT bindings > and the driver. So I think patch [1/2] in this series is fine. You are mixing different things. This patchset creates inconsistency. You even refer here to bindings while we discuss the driver... Why this one driver is different than all other Linux drivers? Why do you keep pushing here entirely different behavior? The devices are compatible, so growing match table is both redundant and confusing. That's everywhere. TPM is not different. Best regards, Krzysztof
On Sat, 2024-04-13 at 10:38 +0200, Krzysztof Kozlowski wrote: > On 13/04/2024 09:15, M. Haener wrote: > > From: Michael Haener <michael.haener@siemens.com> > > > > Add the ST chip st33ktpm2xi2c to the supported compatible strings > > of the > > TPM TIS I2C schema. The Chip is compliant with the TCG PC Client > > TPM > > Profile specification. > > > > For reference, a datasheet is available at: > > https://www.st.com/resource/en/data_brief/st33ktpm2xi2c.pdf > > > > Reviewed-by: Alexander Sverdlin <alexander.sverdlin@siemens.com> > > Signed-off-by: Michael Haener <michael.haener@siemens.com> > > --- > > > Not tested... I was only able to verify and test the conformity of the ST chip st33ktpm2xi2c with kernel 6.1, so I left out the test-by tag. Unfortunately, there is no newer kernel for my embedded hardware. > > Please use scripts/get_maintainers.pl to get a list of necessary > people > and lists to CC. It might happen, that command when run on an older > kernel, gives you outdated entries. Therefore please be sure you base > your patches on recent Linux kernel. > > Tools like b4 or scripts/get_maintainer.pl provide you proper list of > people, so fix your workflow. Tools might also fail if you work on > some > ancient tree (don't, instead use mainline), work on fork of kernel > (don't, instead use mainline) or you ignore some maintainers (really > don't). Just use b4 and everything should be fine, although remember > about `b4 prep --auto-to-cc` if you added new patches to the > patchset. > > You missed at least devicetree list (maybe more), so this won't be > tested by automated tooling. I called the script scripts/get_maintainer.pl on the latest kernel version for each of the two patches and added the output list to the individual patches accordingly. And only for the cover-letter I linked the two lists together. I understand now that I should have sent the whole series to both lists. > > Please kindly resend and include all necessary To/Cc entries. > > Best regards, > Krzysztof > Best regards, Michael
On 13/04/2024 22:26, Haener, Michael wrote: > On Sat, 2024-04-13 at 10:38 +0200, Krzysztof Kozlowski wrote: >> On 13/04/2024 09:15, M. Haener wrote: >>> From: Michael Haener <michael.haener@siemens.com> >>> >>> Add the ST chip st33ktpm2xi2c to the supported compatible strings >>> of the >>> TPM TIS I2C schema. The Chip is compliant with the TCG PC Client >>> TPM >>> Profile specification. >>> >>> For reference, a datasheet is available at: >>> https://www.st.com/resource/en/data_brief/st33ktpm2xi2c.pdf >>> >>> Reviewed-by: Alexander Sverdlin <alexander.sverdlin@siemens.com> >>> Signed-off-by: Michael Haener <michael.haener@siemens.com> >>> --- >> >> >> Not tested... > > I was only able to verify and test the conformity of the ST chip > st33ktpm2xi2c with kernel 6.1, so I left out the test-by tag. I don't mean your tag. Your SoB means you tested it, but I meant you did not send the binding for testing via automation. > Unfortunately, there is no newer kernel for my embedded hardware. > >> >> Please use scripts/get_maintainers.pl to get a list of necessary >> people >> and lists to CC. It might happen, that command when run on an older >> kernel, gives you outdated entries. Therefore please be sure you base >> your patches on recent Linux kernel. >> >> Tools like b4 or scripts/get_maintainer.pl provide you proper list of >> people, so fix your workflow. Tools might also fail if you work on >> some >> ancient tree (don't, instead use mainline), work on fork of kernel >> (don't, instead use mainline) or you ignore some maintainers (really >> don't). Just use b4 and everything should be fine, although remember >> about `b4 prep --auto-to-cc` if you added new patches to the >> patchset. >> >> You missed at least devicetree list (maybe more), so this won't be >> tested by automated tooling. > > I called the script scripts/get_maintainer.pl on the latest kernel > version for each of the two patches and added the output list to the > individual patches accordingly. And only for the cover-letter I linked > the two lists together. > I understand now that I should have sent the whole series to both > lists. > No, that's not the case. You did not Cc output of get_maintainer.pl. Read *AGAIN* my message.. Best regards, Krzysztof
On Sat Apr 13, 2024 at 10:15 AM EEST, M. Haener wrote: > From: Michael Haener <michael.haener@siemens.com> > > Add the ST chip st33ktpm2xi2c to the supported compatible strings of the > TPM TIS I2C schema. The Chip is compliant with the TCG PC Client TPM > Profile specification. > > For reference, a datasheet is available at: > https://www.st.com/resource/en/data_brief/st33ktpm2xi2c.pdf > > Reviewed-by: Alexander Sverdlin <alexander.sverdlin@siemens.com> > Signed-off-by: Michael Haener <michael.haener@siemens.com> > --- > Documentation/devicetree/bindings/tpm/tcg,tpm-tis-i2c.yaml | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/Documentation/devicetree/bindings/tpm/tcg,tpm-tis-i2c.yaml b/Documentation/devicetree/bindings/tpm/tcg,tpm-tis-i2c.yaml > index 3ab4434b7352..af7720dc4a12 100644 > --- a/Documentation/devicetree/bindings/tpm/tcg,tpm-tis-i2c.yaml > +++ b/Documentation/devicetree/bindings/tpm/tcg,tpm-tis-i2c.yaml > @@ -32,6 +32,7 @@ properties: > - enum: > - infineon,slb9673 > - nuvoton,npct75x > + - st,st33ktpm2xi2c > - const: tcg,tpm-tis-i2c > > - description: TPM 1.2 and 2.0 chips with vendor-specific I²C interface Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org> I'm ready to pick these unless Rob has anything to complain (hold to next week). BR, Jarkko
On 13/04/2024 23:47, Jarkko Sakkinen wrote: > On Sat Apr 13, 2024 at 10:15 AM EEST, M. Haener wrote: >> From: Michael Haener <michael.haener@siemens.com> >> >> Add the ST chip st33ktpm2xi2c to the supported compatible strings of the >> TPM TIS I2C schema. The Chip is compliant with the TCG PC Client TPM >> Profile specification. >> >> For reference, a datasheet is available at: >> https://www.st.com/resource/en/data_brief/st33ktpm2xi2c.pdf >> >> Reviewed-by: Alexander Sverdlin <alexander.sverdlin@siemens.com> >> Signed-off-by: Michael Haener <michael.haener@siemens.com> >> --- >> Documentation/devicetree/bindings/tpm/tcg,tpm-tis-i2c.yaml | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/Documentation/devicetree/bindings/tpm/tcg,tpm-tis-i2c.yaml b/Documentation/devicetree/bindings/tpm/tcg,tpm-tis-i2c.yaml >> index 3ab4434b7352..af7720dc4a12 100644 >> --- a/Documentation/devicetree/bindings/tpm/tcg,tpm-tis-i2c.yaml >> +++ b/Documentation/devicetree/bindings/tpm/tcg,tpm-tis-i2c.yaml >> @@ -32,6 +32,7 @@ properties: >> - enum: >> - infineon,slb9673 >> - nuvoton,npct75x >> + - st,st33ktpm2xi2c >> - const: tcg,tpm-tis-i2c >> >> - description: TPM 1.2 and 2.0 chips with vendor-specific I²C interface > > Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org> > > I'm ready to pick these unless Rob has anything to complain (hold > to next week). I complained and NAKed second patch. Best regards, Krzysztof
On Sat Apr 13, 2024 at 11:10 AM EEST, Krzysztof Kozlowski wrote: > On 13/04/2024 09:15, M. Haener wrote: > > From: Michael Haener <michael.haener@siemens.com> > > > > Please use subject prefixes matching the subsystem. You can get them for > example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory > your patch is touching. > > A nit, subject: drop second/last, redundant "binding". The "dt-bindings" > prefix is already stating that these are bindings. > See also: > https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18 > > > > Documentation/devicetree/bindings/tpm/tcg,tpm-tis-i2c.yaml | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/Documentation/devicetree/bindings/tpm/tcg,tpm-tis-i2c.yaml b/Documentation/devicetree/bindings/tpm/tcg,tpm-tis-i2c.yaml > > index 3ab4434b7352..af7720dc4a12 100644 > > --- a/Documentation/devicetree/bindings/tpm/tcg,tpm-tis-i2c.yaml > > +++ b/Documentation/devicetree/bindings/tpm/tcg,tpm-tis-i2c.yaml > > @@ -32,6 +32,7 @@ properties: > > - enum: > > - infineon,slb9673 > > - nuvoton,npct75x > > + - st,st33ktpm2xi2c > > I got only one patch, but if these are compatible, why do you need > second patch? Plus binding come before users. > > Please explain what are you doing in other patch. > > Best regards, > Krzysztof Thanks, I agree with the remarks. So holding for update. BR, Jarkko
On 13/04/2024 23:47, Jarkko Sakkinen wrote: > On Sat Apr 13, 2024 at 10:15 AM EEST, M. Haener wrote: >> From: Michael Haener <michael.haener@siemens.com> >> >> Add the ST chip st33ktpm2xi2c to the supported compatible strings of the >> TPM TIS I2C schema. The Chip is compliant with the TCG PC Client TPM >> Profile specification. >> >> For reference, a datasheet is available at: >> https://www.st.com/resource/en/data_brief/st33ktpm2xi2c.pdf >> >> Reviewed-by: Alexander Sverdlin <alexander.sverdlin@siemens.com> >> Signed-off-by: Michael Haener <michael.haener@siemens.com> >> --- >> Documentation/devicetree/bindings/tpm/tcg,tpm-tis-i2c.yaml | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/Documentation/devicetree/bindings/tpm/tcg,tpm-tis-i2c.yaml b/Documentation/devicetree/bindings/tpm/tcg,tpm-tis-i2c.yaml >> index 3ab4434b7352..af7720dc4a12 100644 >> --- a/Documentation/devicetree/bindings/tpm/tcg,tpm-tis-i2c.yaml >> +++ b/Documentation/devicetree/bindings/tpm/tcg,tpm-tis-i2c.yaml >> @@ -32,6 +32,7 @@ properties: >> - enum: >> - infineon,slb9673 >> - nuvoton,npct75x >> + - st,st33ktpm2xi2c >> - const: tcg,tpm-tis-i2c >> >> - description: TPM 1.2 and 2.0 chips with vendor-specific I²C interface > > Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org> > > I'm ready to pick these unless Rob has anything to complain (hold > to next week). Plus, even the first patch was not tested... I already wrote it, so let's be more specific: NAK, not tested, even though testing is trivial. I don't understand why opting-out from testing, even for trivial patches like this. Automation, expressed by get_maintainers.pl, is there for a reason. Best regards, Krzysztof
On Sat, 2024-04-13 at 13:01 +0200, Krzysztof Kozlowski wrote: > On 13/04/2024 12:56, Lukas Wunner wrote: > > On Sat, Apr 13, 2024 at 12:53:25PM +0200, Krzysztof Kozlowski > > wrote: > > > On 13/04/2024 12:51, Lukas Wunner wrote: > > > > The binding requires two entries in the compatible string used > > > > in the DT, > > > > the chip name followed by the generic string: > > > > > > > > items: > > > > - enum: > > > > - infineon,slb9673 > > > > - nuvoton,npct75x > > > > - const: tcg,tpm-tis-i2c > > > > > > > > This allows us to deal with device-specific quirks, should they > > > > pop up > > > > (e.g. special timing requirements, hardware bugs). We don't > > > > know in > > > > advance if they will be discovered, but if they are, it's > > > > cumbersome > > > > to determine after the fact which products (and thus DTs) are > > > > affected. > > > > So having the name of the actual chip used on the board has > > > > value. > > > > > > So you say devices are compatible. Then the second patch is > > > wrong. > > > > > > I cannot respond to it, though... so NAK-here-for-second-patch. > > > > I disagree. It's ugly to have inconsistencies between the DT > > bindings > > and the driver. So I think patch [1/2] in this series is fine. > > You are mixing different things. This patchset creates inconsistency. > You even refer here to bindings while we discuss the driver... > > Why this one driver is different than all other Linux drivers? Why do > you keep pushing here entirely different behavior? > > The devices are compatible, so growing match table is both redundant > and > confusing. That's everywhere. TPM is not different. > > Best regards, > Krzysztof > Sorry for the mistakes made (kernel noob). I made the patch for the driver analogous to the i2c-tpm of infineon,slb9673 and nuvoton,npct75x. In the discussion I realize that the compatibility is a duplication and should not be extended. I now adjust the following in the series: - correct cc receiver list - delete driver patch with compatibility extension - change commit title to "dt-bindings: tpm: Add st,st33ktpm2xi2c" Best regards, Michael
On 14/04/2024 07:42, Haener, Michael wrote: > On Sat, 2024-04-13 at 13:01 +0200, Krzysztof Kozlowski wrote: >> On 13/04/2024 12:56, Lukas Wunner wrote: >>> On Sat, Apr 13, 2024 at 12:53:25PM +0200, Krzysztof Kozlowski >>> wrote: >>>> On 13/04/2024 12:51, Lukas Wunner wrote: >>>>> The binding requires two entries in the compatible string used >>>>> in the DT, >>>>> the chip name followed by the generic string: >>>>> >>>>> items: >>>>> - enum: >>>>> - infineon,slb9673 >>>>> - nuvoton,npct75x >>>>> - const: tcg,tpm-tis-i2c >>>>> >>>>> This allows us to deal with device-specific quirks, should they >>>>> pop up >>>>> (e.g. special timing requirements, hardware bugs). We don't >>>>> know in >>>>> advance if they will be discovered, but if they are, it's >>>>> cumbersome >>>>> to determine after the fact which products (and thus DTs) are >>>>> affected. >>>>> So having the name of the actual chip used on the board has >>>>> value. >>>> >>>> So you say devices are compatible. Then the second patch is >>>> wrong. >>>> >>>> I cannot respond to it, though... so NAK-here-for-second-patch. >>> >>> I disagree. It's ugly to have inconsistencies between the DT >>> bindings >>> and the driver. So I think patch [1/2] in this series is fine. >> >> You are mixing different things. This patchset creates inconsistency. >> You even refer here to bindings while we discuss the driver... >> >> Why this one driver is different than all other Linux drivers? Why do >> you keep pushing here entirely different behavior? >> >> The devices are compatible, so growing match table is both redundant >> and >> confusing. That's everywhere. TPM is not different. >> >> Best regards, >> Krzysztof >> > > Sorry for the mistakes made (kernel noob). > I made the patch for the driver analogous to the i2c-tpm of > infineon,slb9673 and nuvoton,npct75x. In the discussion I realize that > the compatibility is a duplication and should not be extended. > > I now adjust the following in the series: > - correct cc receiver list > - delete driver patch with compatibility extension > - change commit title to "dt-bindings: tpm: Add st,st33ktpm2xi2c" Thanks. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/tpm/tcg,tpm-tis-i2c.yaml b/Documentation/devicetree/bindings/tpm/tcg,tpm-tis-i2c.yaml index 3ab4434b7352..af7720dc4a12 100644 --- a/Documentation/devicetree/bindings/tpm/tcg,tpm-tis-i2c.yaml +++ b/Documentation/devicetree/bindings/tpm/tcg,tpm-tis-i2c.yaml @@ -32,6 +32,7 @@ properties: - enum: - infineon,slb9673 - nuvoton,npct75x + - st,st33ktpm2xi2c - const: tcg,tpm-tis-i2c - description: TPM 1.2 and 2.0 chips with vendor-specific I²C interface