| Message ID | 20241028053220.346283-1-TroyMitchell988@gmail.com |
|---|---|
| Headers | show |
| Series | riscv: spacemit: add i2c support to K1 SoC | expand |
On Mon, Oct 28, 2024 at 01:32:19PM +0800, Troy Mitchell wrote: > The I2C of K1 supports fast-speed-mode and high-speed-mode, > and supports FIFO transmission. > > Signed-off-by: Troy Mitchell <TroyMitchell988@gmail.com> > --- Where is the changelog? Nothing here, nothing in cover letter. I asked for several changes, so now I don't know if you implemented them. Best regards, Krzysztof
On 2024/10/28 15:38, Krzysztof Kozlowski wrote: > On Mon, Oct 28, 2024 at 01:32:19PM +0800, Troy Mitchell wrote: >> The I2C of K1 supports fast-speed-mode and high-speed-mode, >> and supports FIFO transmission. >> >> Signed-off-by: Troy Mitchell <TroyMitchell988@gmail.com> >> --- > > Where is the changelog? Nothing here, nothing in cover letter. > > I asked for several changes, so now I don't know if you implemented > them. I deleted the FIFO property because I believe your suggestion is correct. this should be decided by the driver, even though the FIFO is provided by the hardware. Apologies for missing the changelog. To correct this, should I send a v3 version with the changelog or resend v2? > > Best regards, > Krzysztof >
On Tue, Oct 29, 2024 at 04:36:00PM +0800, Troy Mitchell wrote: > On 2024/10/28 15:38, Krzysztof Kozlowski wrote: > > On Mon, Oct 28, 2024 at 01:32:19PM +0800, Troy Mitchell wrote: > >> The I2C of K1 supports fast-speed-mode and high-speed-mode, > >> and supports FIFO transmission. > >> > >> Signed-off-by: Troy Mitchell <TroyMitchell988@gmail.com> > >> --- > > > > Where is the changelog? Nothing here, nothing in cover letter. > > > > I asked for several changes, so now I don't know if you implemented > > them. > > I deleted the FIFO property because I believe your suggestion is correct. > this should be decided by the driver, even though the FIFO is provided > by the hardware. > > Apologies for missing the changelog. To correct this, should I send a v3 > version with the changelog or resend v2? Reply now with changelog. Your binding has some other unrelated and incorrect changes, which I do not understand. Best regards, Krzysztof
On Mon, Oct 28, 2024 at 01:32:19PM +0800, Troy Mitchell wrote: > The I2C of K1 supports fast-speed-mode and high-speed-mode, > and supports FIFO transmission. > > Signed-off-by: Troy Mitchell <TroyMitchell988@gmail.com> > --- > .../bindings/i2c/spacemit,k1-i2c.yaml | 51 +++++++++++++++++++ > 1 file changed, 51 insertions(+) > create mode 100644 Documentation/devicetree/bindings/i2c/spacemit,k1-i2c.yaml > > diff --git a/Documentation/devicetree/bindings/i2c/spacemit,k1-i2c.yaml b/Documentation/devicetree/bindings/i2c/spacemit,k1-i2c.yaml > new file mode 100644 > index 000000000000..57af66f494e7 > --- /dev/null > +++ b/Documentation/devicetree/bindings/i2c/spacemit,k1-i2c.yaml > @@ -0,0 +1,51 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/i2c/spacemit,k1-i2c.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: I2C controller embedded in SpacemiT's K1 SoC > + > +maintainers: > + - Troy Mitchell <troymitchell988@gmail.com> > + > +properties: > + compatible: > + const: spacemit,k1-i2c > + > + reg: > + maxItems: 2 Provide changelog explaining why you are doing some changes. List and describe items here instead. > + > + interrupts: > + maxItems: 1 > + > + clocks: > + maxItems: 1 > + > + clock-frequency: > + description: > + Desired I2C bus clock frequency in Hz. As only fast and high-speed > + modes are supported by hardware, possible values are 100000 and 400000. > + enum: [100000, 400000] > + default: 100000 > + > +required: > + - compatible > + - reg > + - interrupts > + - clocks > + > +unevaluatedProperties: false > + > +examples: > + - | > + i2c@d4010800 { > + compatible = "spacemit,k1-i2c"; > + reg = <0x0 0xd4010800 0x0 0x38>; Missing second item. Best regards, Krzysztof
Hi Tony, On Mon, Oct 28, 2024 at 01:32:18PM +0800, Troy Mitchell wrote: > Hi all, > > This patch implements I2C driver for the SpacemiT K1 SoC, > providing basic support for I2C read/write communication which > compatible with standard I2C bus specifications. > > In this version, the driver defaults to use fast-speed-mode and > interrupts for transmission, and does not support DMA, high-speed mode, or FIFO. > > The docs of I2C can be found here, in chapter 16.1 I2C [1] > > Link: https://developer.spacemit.com/documentation?token=Rn9Kw3iFHirAMgkIpTAcV2Arnkf#part5 [1] > > Troy Mitchell (2): > dt-bindings: i2c: spacemit: add support for K1 SoC > i2c: spacemit: add support for SpacemiT K1 SoC As Krzysztof has asked, please do provide the changelog, it's important to track the progress of your series. Thanks, Andi > .../bindings/i2c/spacemit,k1-i2c.yaml | 51 ++ > drivers/i2c/busses/Kconfig | 18 + > drivers/i2c/busses/Makefile | 1 + > drivers/i2c/busses/i2c-k1.c | 658 ++++++++++++++++++ > 4 files changed, 728 insertions(+) > create mode 100644 Documentation/devicetree/bindings/i2c/spacemit,k1-i2c.yaml > create mode 100644 drivers/i2c/busses/i2c-k1.c > > -- > 2.34.1 >
On Fri, Nov 1, 2024 at 10:24 AM Krzysztof Kozlowski <krzk@kernel.org> wrote: > > On Tue, Oct 29, 2024 at 04:36:00PM +0800, Troy Mitchell wrote: > > On 2024/10/28 15:38, Krzysztof Kozlowski wrote: > > > On Mon, Oct 28, 2024 at 01:32:19PM +0800, Troy Mitchell wrote: > > >> The I2C of K1 supports fast-speed-mode and high-speed-mode, > > >> and supports FIFO transmission. > > >> > > >> Signed-off-by: Troy Mitchell <TroyMitchell988@gmail.com> > > >> --- Change in v2: - drop fifo-disable property - unevaluatedProperties goes after required: block - drop alias > > > > > > Where is the changelog? Nothing here, nothing in cover letter. It seems like it was accidentally dropped seeing there are the dashes. > > > > > > I asked for several changes, so now I don't know if you implemented > > > them. > > > > I deleted the FIFO property because I believe your suggestion is correct. > > this should be decided by the driver, even though the FIFO is provided > > by the hardware. > > > > Apologies for missing the changelog. To correct this, should I send a v3 > > version with the changelog or resend v2? > > Reply now with changelog. Your binding has some other unrelated and > incorrect changes, which I do not understand. ... Thanks, Jesse Taube > > Best regards, > Krzysztof > > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv
On 2024/10/31 19:43, Andi Shyti wrote: > Hi Tony, > > On Mon, Oct 28, 2024 at 01:32:18PM +0800, Troy Mitchell wrote: >> Hi all, >> >> This patch implements I2C driver for the SpacemiT K1 SoC, >> providing basic support for I2C read/write communication which >> compatible with standard I2C bus specifications. >> >> In this version, the driver defaults to use fast-speed-mode and >> interrupts for transmission, and does not support DMA, high-speed mode, or FIFO. >> >> The docs of I2C can be found here, in chapter 16.1 I2C [1] >> >> Link: https://developer.spacemit.com/documentation?token=Rn9Kw3iFHirAMgkIpTAcV2Arnkf#part5 [1] >> >> Troy Mitchell (2): >> dt-bindings: i2c: spacemit: add support for K1 SoC >> i2c: spacemit: add support for SpacemiT K1 SoC > > As Krzysztof has asked, please do provide the changelog, it's > important to track the progress of your series. I saw a compilation warning sent to me by the robot, and I've fixed the warning. Should I resend V2 with the changelog what I miss or send V3? Thank for your response. > > Thanks, > Andi > >> .../bindings/i2c/spacemit,k1-i2c.yaml | 51 ++ >> drivers/i2c/busses/Kconfig | 18 + >> drivers/i2c/busses/Makefile | 1 + >> drivers/i2c/busses/i2c-k1.c | 658 ++++++++++++++++++ >> 4 files changed, 728 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/i2c/spacemit,k1-i2c.yaml >> create mode 100644 drivers/i2c/busses/i2c-k1.c >> >> -- >> 2.34.1 >>
On 2024/10/31 16:00, Krzysztof Kozlowski wrote: > On Tue, Oct 29, 2024 at 04:36:00PM +0800, Troy Mitchell wrote: >> On 2024/10/28 15:38, Krzysztof Kozlowski wrote: >>> On Mon, Oct 28, 2024 at 01:32:19PM +0800, Troy Mitchell wrote: >>>> The I2C of K1 supports fast-speed-mode and high-speed-mode, >>>> and supports FIFO transmission. >>>> >>>> Signed-off-by: Troy Mitchell <TroyMitchell988@gmail.com> >>>> --- Change in v2: - Change the maxItems of reg from 1 to 2 in properties - Change 'i2c' to 'I2C' in the commit message. - Drop fifo-disable property - Drop alias in dts example - Move `unevaluatedProperties` after `required:` block >>> >>> Where is the changelog? Nothing here, nothing in cover letter. >>> >>> I asked for several changes, so now I don't know if you implemented >>> them. >> >> I deleted the FIFO property because I believe your suggestion is correct. >> this should be decided by the driver, even though the FIFO is provided >> by the hardware. >> >> Apologies for missing the changelog. To correct this, should I send a v3 >> version with the changelog or resend v2? > > Reply now with changelog. Your binding has some other unrelated and > incorrect changes, which I do not understand. > > Best regards, > Krzysztof >
On 04/11/2024 14:01, Troy Mitchell wrote: > > On 2024/10/31 16:00, Krzysztof Kozlowski wrote: >> On Tue, Oct 29, 2024 at 04:36:00PM +0800, Troy Mitchell wrote: >>> On 2024/10/28 15:38, Krzysztof Kozlowski wrote: >>>> On Mon, Oct 28, 2024 at 01:32:19PM +0800, Troy Mitchell wrote: >>>>> The I2C of K1 supports fast-speed-mode and high-speed-mode, >>>>> and supports FIFO transmission. >>>>> >>>>> Signed-off-by: Troy Mitchell <TroyMitchell988@gmail.com> >>>>> --- > Change in v2: > - Change the maxItems of reg from 1 to 2 in properties Why? > - Change 'i2c' to 'I2C' in the commit message. > - Drop fifo-disable property > - Drop alias in dts example > - Move `unevaluatedProperties` after `required:` block Rest look ok. Best regards, Krzysztof
On 2024/11/4 22:48, Krzysztof Kozlowski wrote: > On 04/11/2024 14:01, Troy Mitchell wrote: >> >> On 2024/10/31 16:00, Krzysztof Kozlowski wrote: >>> On Tue, Oct 29, 2024 at 04:36:00PM +0800, Troy Mitchell wrote: >>>> On 2024/10/28 15:38, Krzysztof Kozlowski wrote: >>>>> On Mon, Oct 28, 2024 at 01:32:19PM +0800, Troy Mitchell wrote: >>>>>> The I2C of K1 supports fast-speed-mode and high-speed-mode, >>>>>> and supports FIFO transmission. >>>>>> >>>>>> Signed-off-by: Troy Mitchell <TroyMitchell988@gmail.com> >>>>>> --- >> Change in v2: >> - Change the maxItems of reg from 1 to 2 in properties > > Why? I need the address and size. In v1, I wrote it as 1, and I got the make dt_binding_check error. > >> - Change 'i2c' to 'I2C' in the commit message. >> - Drop fifo-disable property >> - Drop alias in dts example >> - Move `unevaluatedProperties` after `required:` block > > Rest look ok. > > Best regards, > Krzysztof >
Hi Troy, On 2024-11-04 7:14 PM, Troy Mitchell wrote: > > On 2024/11/4 22:48, Krzysztof Kozlowski wrote: >> On 04/11/2024 14:01, Troy Mitchell wrote: >>> >>> On 2024/10/31 16:00, Krzysztof Kozlowski wrote: >>>> On Tue, Oct 29, 2024 at 04:36:00PM +0800, Troy Mitchell wrote: >>>>> On 2024/10/28 15:38, Krzysztof Kozlowski wrote: >>>>>> On Mon, Oct 28, 2024 at 01:32:19PM +0800, Troy Mitchell wrote: >>>>>>> The I2C of K1 supports fast-speed-mode and high-speed-mode, >>>>>>> and supports FIFO transmission. >>>>>>> >>>>>>> Signed-off-by: Troy Mitchell <TroyMitchell988@gmail.com> >>>>>>> --- >>> Change in v2: >>> - Change the maxItems of reg from 1 to 2 in properties >> >> Why? > I need the address and size. In v1, I wrote it as 1, and I got the make > dt_binding_check error. One "<address size>" element is just one item. maxItems > 1 would be for hardware with multiple discontiguous register ranges: <address1 size1>, <address2 size2>. Regards, Samuel
On 2024/11/5 10:50, Samuel Holland wrote: > One "<address size>" element is just one item. maxItems > 1 would be for > hardware with multiple discontiguous register ranges: <address1 size1>, > <address2 size2>. got it.I will fix it in next version. > > Regards, > Samuel
Hy Troy, On Mon, Nov 04, 2024 at 08:23:23PM +0800, Troy Mitchell wrote: > On 2024/10/31 19:43, Andi Shyti wrote: > > Hi Tony, Sorry, I misread your name :-/ > > On Mon, Oct 28, 2024 at 01:32:18PM +0800, Troy Mitchell wrote: > >> Hi all, > >> > >> This patch implements I2C driver for the SpacemiT K1 SoC, > >> providing basic support for I2C read/write communication which > >> compatible with standard I2C bus specifications. > >> > >> In this version, the driver defaults to use fast-speed-mode and > >> interrupts for transmission, and does not support DMA, high-speed mode, or FIFO. > >> > >> The docs of I2C can be found here, in chapter 16.1 I2C [1] > >> > >> Link: https://developer.spacemit.com/documentation?token=Rn9Kw3iFHirAMgkIpTAcV2Arnkf#part5 [1] > >> > >> Troy Mitchell (2): > >> dt-bindings: i2c: spacemit: add support for K1 SoC > >> i2c: spacemit: add support for SpacemiT K1 SoC > > > > As Krzysztof has asked, please do provide the changelog, it's > > important to track the progress of your series. > I saw a compilation warning sent to me by the robot, and I've > fixed the warning. Should I resend V2 with the changelog > what I miss or send V3? Please send a v3. When there are compilation issues, normally patches are less keen to be reviewed. You can add the changelog in the Patch 0/2 to avoid editing all the .patch files. Thanks, Andi
On 2024/11/5 22:21, Andi Shyti wrote: > Hy Troy, > > On Mon, Nov 04, 2024 at 08:23:23PM +0800, Troy Mitchell wrote: >> On 2024/10/31 19:43, Andi Shyti wrote: >>> Hi Tony, > > Sorry, I misread your name :-/ It doesn't matter > >>> On Mon, Oct 28, 2024 at 01:32:18PM +0800, Troy Mitchell wrote: >>>> Hi all, >>>> >>>> This patch implements I2C driver for the SpacemiT K1 SoC, >>>> providing basic support for I2C read/write communication which >>>> compatible with standard I2C bus specifications. >>>> >>>> In this version, the driver defaults to use fast-speed-mode and >>>> interrupts for transmission, and does not support DMA, high-speed mode, or FIFO. >>>> >>>> The docs of I2C can be found here, in chapter 16.1 I2C [1] >>>> >>>> Link: https://developer.spacemit.com/documentation?token=Rn9Kw3iFHirAMgkIpTAcV2Arnkf#part5 [1] >>>> >>>> Troy Mitchell (2): >>>> dt-bindings: i2c: spacemit: add support for K1 SoC >>>> i2c: spacemit: add support for SpacemiT K1 SoC >>> >>> As Krzysztof has asked, please do provide the changelog, it's >>> important to track the progress of your series. >> I saw a compilation warning sent to me by the robot, and I've >> fixed the warning. Should I resend V2 with the changelog >> what I miss or send V3? > > Please send a v3. When there are compilation issues, normally > patches are less keen to be reviewed. > > You can add the changelog in the Patch 0/2 to avoid editing all > the .patch files. Thanks! I'll send v3 after I get a response from Samuel. > > Thanks, > Andi