This patch series adds a set of drivers for operating the Sense HAT
peripheral device. This board is an add on for the Raspberry Pi that is
designed to connect using the GPIO connector and communicate via I2C.

It features:
	- a joystick
	- an 8x8 RGB LED matrix display
	- a whole bunch of environmental sensors with their own drivers
	  (those are already in upstream Linux)

This is a refactor of the work of Serge Schneider, the author of a
version of this driver that is currently in the Raspberry Pi downstream
kernel. We modified his code to make it suitable for upstream Linux.

A couple of tests are available for the driver in the test folder in
this repo: https://github.com/underground-software/sensehat.git
	- sensehat_joystick_test logs the input events from the
	  joystick to the console
	- sensehat_display_test displays various solid colors on
	  the LED panel.
	- full_sensehat_test displays a single lit cell that can be
	  moved with the joystick. Pressing the joystick ends the
	  program.

For more information about the Sense HAT, visit:
https://www.raspberrypi.org/products/sense-hat/

Changes since v9:
	- The driver for the sensehat joystick was accepted upstream so
	  it is no longer included in this patchset.
	- Some of the people involved in the development of earlier
	  versions of these patches are no longer involved in the project
	  so they have been removed from the list of maintainers and the
	  patches.
	- The code is completely unchanged and so based on the discussions
	  from version 9 of the patches, it should be ready to be merged.

My appologies for the long delay between v9 and v10 especially given the
fact that there are no changes to the code. We appreciate the maintainers'
patience and guidance throughout all 10 versions of this sensehat patchset,
and we are excited for this driver to hopefully be fully accepted.

Best - Charlie

Co-developed-by: Joel Slebodnick <jslebodn@redhat.com>
Signed-off-by: Joel Slebodnick <jslebodn@redhat.com>
Co-developed-by: Joel Savitz <jsavitz@redhat.com>
Signed-off-by: Joel Savitz <jsavitz@redhat.com>
Signed-off-by: Charles Mirabile <cmirabil@redhat.com>

Charles Mirabile (5):
  drivers/mfd: sensehat: Add Raspberry Pi Sense HAT to simple_mfd_i2c
  drivers/auxdisplay: sensehat: Raspberry Pi Sense HAT display driver
  dt-bindings: mfd: sensehat: Add Raspberry Pi Sense HAT schema
  MAINTAINERS: Add sensehat driver authors to MAINTAINERS
  DO NOT MERGE: full sensehat device tree overlay for raspberry pi 4

 .../raspberrypi,sensehat-display.yaml         |  26 +++
 .../input/raspberrypi,sensehat-joystick.yaml  |  31 +++
 .../bindings/mfd/raspberrypi,sensehat.yaml    |  57 +++++
 MAINTAINERS                                   |  10 +
 drivers/auxdisplay/Kconfig                    |   8 +
 drivers/auxdisplay/Makefile                   |   1 +
 drivers/auxdisplay/sensehat-display.c         | 205 ++++++++++++++++++
 drivers/mfd/simple-mfd-i2c.c                  |   1 +
 sensehat.dtbs                                 |  52 +++++
 9 files changed, 391 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/auxdisplay/raspberrypi,sensehat-display.yaml
 create mode 100644 Documentation/devicetree/bindings/input/raspberrypi,sensehat-joystick.yaml
 create mode 100644 Documentation/devicetree/bindings/mfd/raspberrypi,sensehat.yaml
 create mode 100644 drivers/auxdisplay/sensehat-display.c
 create mode 100644 sensehat.dtbs

-- 
2.31.1

Hi Charles,

+ Maxime & Noralf

Am 23.08.22 um 19:41 schrieb Charles Mirabile:
> This patch series adds a set of drivers for operating the Sense HAT
> peripheral device. This board is an add on for the Raspberry Pi that is
> designed to connect using the GPIO connector and communicate via I2C.
thanks for sending a new version of this series.
>
> It features:
> 	- a joystick
> 	- an 8x8 RGB LED matrix display
> 	- a whole bunch of environmental sensors with their own drivers
> 	  (those are already in upstream Linux)
>
> This is a refactor of the work of Serge Schneider, the author of a
> version of this driver that is currently in the Raspberry Pi downstream
> kernel. We modified his code to make it suitable for upstream Linux.
>
> A couple of tests are available for the driver in the test folder in
> this repo: https://github.com/underground-software/sensehat.git
> 	- sensehat_joystick_test logs the input events from the
> 	  joystick to the console
> 	- sensehat_display_test displays various solid colors on
> 	  the LED panel.
> 	- full_sensehat_test displays a single lit cell that can be
> 	  moved with the joystick. Pressing the joystick ends the
> 	  program.
>
> For more information about the Sense HAT, visit:
> https://www.raspberrypi.org/products/sense-hat/
>
> Changes since v9:
> 	- The driver for the sensehat joystick was accepted upstream so
> 	  it is no longer included in this patchset.
> 	- Some of the people involved in the development of earlier
> 	  versions of these patches are no longer involved in the project
> 	  so they have been removed from the list of maintainers and the
> 	  patches.
> 	- The code is completely unchanged and so based on the discussions
> 	  from version 9 of the patches, it should be ready to be merged.
>
> My appologies for the long delay between v9 and v10 especially given the
> fact that there are no changes to the code. We appreciate the maintainers'
> patience and guidance throughout all 10 versions of this sensehat patchset,
> and we are excited for this driver to hopefully be fully accepted.
>
> Best - Charlie
>
> Co-developed-by: Joel Slebodnick <jslebodn@redhat.com>
> Signed-off-by: Joel Slebodnick <jslebodn@redhat.com>
> Co-developed-by: Joel Savitz <jsavitz@redhat.com>
> Signed-off-by: Joel Savitz <jsavitz@redhat.com>
> Signed-off-by: Charles Mirabile <cmirabil@redhat.com>
>
> Charles Mirabile (5):
>    drivers/mfd: sensehat: Add Raspberry Pi Sense HAT to simple_mfd_i2c
AFAIK Lee Jones has a new kernel.org address the old linaro address 
shouldn't be used anymore. We don't get any further without him.
>    drivers/auxdisplay: sensehat: Raspberry Pi Sense HAT display driver
We don't have an Ack for this display driver. Maybe we should ask Maxime 
Ripard and Noralf Trønnes especially Pavel Machek had concerns about the 
right subsystem?
>    dt-bindings: mfd: sensehat: Add Raspberry Pi Sense HAT schema
>    MAINTAINERS: Add sensehat driver authors to MAINTAINERS
>    DO NOT MERGE: full sensehat device tree overlay for raspberry pi 4

In case you want to send a new version it would be nice to enable the 
driver in bcm2835_defconfig for better test coverage. But this is not a 
show stopper.

Best regards

>
>   .../raspberrypi,sensehat-display.yaml         |  26 +++
>   .../input/raspberrypi,sensehat-joystick.yaml  |  31 +++
>   .../bindings/mfd/raspberrypi,sensehat.yaml    |  57 +++++
>   MAINTAINERS                                   |  10 +
>   drivers/auxdisplay/Kconfig                    |   8 +
>   drivers/auxdisplay/Makefile                   |   1 +
>   drivers/auxdisplay/sensehat-display.c         | 205 ++++++++++++++++++
>   drivers/mfd/simple-mfd-i2c.c                  |   1 +
>   sensehat.dtbs                                 |  52 +++++
>   9 files changed, 391 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/auxdisplay/raspberrypi,sensehat-display.yaml
>   create mode 100644 Documentation/devicetree/bindings/input/raspberrypi,sensehat-joystick.yaml
>   create mode 100644 Documentation/devicetree/bindings/mfd/raspberrypi,sensehat.yaml
>   create mode 100644 drivers/auxdisplay/sensehat-display.c
>   create mode 100644 sensehat.dtbs
>

On Tue, Aug 23, 2022 at 5:30 PM Stefan Wahren <stefan.wahren@i2se.com> wrote:
>
> Hi Charles,
>
> + Maxime & Noralf
>
> Am 23.08.22 um 19:41 schrieb Charles Mirabile:
> > This patch series adds a set of drivers for operating the Sense HAT
> > peripheral device. This board is an add on for the Raspberry Pi that is
> > designed to connect using the GPIO connector and communicate via I2C.
> thanks for sending a new version of this series.
> >
> > It features:
> >       - a joystick
> >       - an 8x8 RGB LED matrix display
> >       - a whole bunch of environmental sensors with their own drivers
> >         (those are already in upstream Linux)
> >
> > This is a refactor of the work of Serge Schneider, the author of a
> > version of this driver that is currently in the Raspberry Pi downstream
> > kernel. We modified his code to make it suitable for upstream Linux.
> >
> > A couple of tests are available for the driver in the test folder in
> > this repo: https://github.com/underground-software/sensehat.git
> >       - sensehat_joystick_test logs the input events from the
> >         joystick to the console
> >       - sensehat_display_test displays various solid colors on
> >         the LED panel.
> >       - full_sensehat_test displays a single lit cell that can be
> >         moved with the joystick. Pressing the joystick ends the
> >         program.
> >
> > For more information about the Sense HAT, visit:
> > https://www.raspberrypi.org/products/sense-hat/
> >
> > Changes since v9:
> >       - The driver for the sensehat joystick was accepted upstream so
> >         it is no longer included in this patchset.
> >       - Some of the people involved in the development of earlier
> >         versions of these patches are no longer involved in the project
> >         so they have been removed from the list of maintainers and the
> >         patches.
> >       - The code is completely unchanged and so based on the discussions
> >         from version 9 of the patches, it should be ready to be merged.
> >
> > My appologies for the long delay between v9 and v10 especially given the
> > fact that there are no changes to the code. We appreciate the maintainers'
> > patience and guidance throughout all 10 versions of this sensehat patchset,
> > and we are excited for this driver to hopefully be fully accepted.
> >
> > Best - Charlie
> >
> > Co-developed-by: Joel Slebodnick <jslebodn@redhat.com>
> > Signed-off-by: Joel Slebodnick <jslebodn@redhat.com>
> > Co-developed-by: Joel Savitz <jsavitz@redhat.com>
> > Signed-off-by: Joel Savitz <jsavitz@redhat.com>
> > Signed-off-by: Charles Mirabile <cmirabil@redhat.com>
> >
> > Charles Mirabile (5):
> >    drivers/mfd: sensehat: Add Raspberry Pi Sense HAT to simple_mfd_i2c
> AFAIK Lee Jones has a new kernel.org address the old linaro address
> shouldn't be used anymore. We don't get any further without him.
oops - my bad. Copied the old one from the previous CC list, but I see
the new one was being output properly by get_maintainer.pl, I just
wasn't paying enough attention
> >    drivers/auxdisplay: sensehat: Raspberry Pi Sense HAT display driver
> We don't have an Ack for this display driver. Maybe we should ask Maxime
> Ripard and Noralf Trønnes especially Pavel Machek had concerns about the
> right subsystem?
I just looked back at the comments from Pavel on v9. I had meant to
reply to him, and I actually drafted something but it got lost in the
shuffle and I never sent it (also my bad).
The driver is an fbdev in the raspberry pi tree, and that is where we
started when upstreaming, but right from the outset we were told that
it couldn't be an fbdev since that subsystem is being deprecated
the replacement is the DRM subsystem, but (as Matthias Brugger
suggests) that is probably overkill for an 8x8 display. Does that
settle the subsystem question?
Also, Is Matthias's review acceptable, or do we need someone else's
ack? It was my understanding based on his comments on v9 that Miguel
Ojeda was also basically ready to ack it, but I don't want to speak
for him.
> >    dt-bindings: mfd: sensehat: Add Raspberry Pi Sense HAT schema
> >    MAINTAINERS: Add sensehat driver authors to MAINTAINERS
> >    DO NOT MERGE: full sensehat device tree overlay for raspberry pi 4
>
> In case you want to send a new version it would be nice to enable the
> driver in bcm2835_defconfig for better test coverage. But this is not a
> show stopper.
Since I am going to have to send a v11 to fix Lee's email address I
will look into this. Is it as simple as adding another patch that puts
CONFIG_SENSEHAT_DISPLAY=y into the next version?
Presumably I would also want to enable CONFIG_JOYSTICK_SENSEHAT right?
Do I have to do anything special since both of those options have
decencies (namely CONFIG_I2C for both and CONFIG_INPUT for the latter,
and both select MFD_SIMPLE_MFD_I2C)?
>
> Best regards
>
> >
> >   .../raspberrypi,sensehat-display.yaml         |  26 +++
> >   .../input/raspberrypi,sensehat-joystick.yaml  |  31 +++
> >   .../bindings/mfd/raspberrypi,sensehat.yaml    |  57 +++++
> >   MAINTAINERS                                   |  10 +
> >   drivers/auxdisplay/Kconfig                    |   8 +
> >   drivers/auxdisplay/Makefile                   |   1 +
> >   drivers/auxdisplay/sensehat-display.c         | 205 ++++++++++++++++++
> >   drivers/mfd/simple-mfd-i2c.c                  |   1 +
> >   sensehat.dtbs                                 |  52 +++++
> >   9 files changed, 391 insertions(+)
> >   create mode 100644 Documentation/devicetree/bindings/auxdisplay/raspberrypi,sensehat-display.yaml
> >   create mode 100644 Documentation/devicetree/bindings/input/raspberrypi,sensehat-joystick.yaml
> >   create mode 100644 Documentation/devicetree/bindings/mfd/raspberrypi,sensehat.yaml
> >   create mode 100644 drivers/auxdisplay/sensehat-display.c
> >   create mode 100644 sensehat.dtbs
> >
>

Am 24.08.22 um 14:06 schrieb Charles Mirabile:
>> In case you want to send a new version it would be nice to enable the
>> driver in bcm2835_defconfig for better test coverage. But this is not a
>> show stopper.
> Since I am going to have to send a v11 to fix Lee's email address I
> will look into this. Is it as simple as adding another patch that puts
> CONFIG_SENSEHAT_DISPLAY=y into the next version?
> Presumably I would also want to enable CONFIG_JOYSTICK_SENSEHAT right?
> Do I have to do anything special since both of those options have
> decencies (namely CONFIG_I2C for both and CONFIG_INPUT for the latter,
> and both select MFD_SIMPLE_MFD_I2C)?

make bcm2835_defconfig

enable both driver via make menuconfig

make savedefconfig

mv defconfig arch/arm/configs/bcm2835_defconfig.2

diff -u arch/arm/configs/bcm2835_defconfig 
arch/arm/configs/bcm2835_defconfig.2 > config.patch

manually add only the relevant changes from config.patch in 
arch/arm/configs/bcm2835_defconfig

generate the patch as usual

goal is to have simple patch which only contains the 2 driver changes 
without any clean-up changes

On Wed, Aug 24, 2022 at 08:06:59AM -0400, Charles Mirabile wrote:
> On Tue, Aug 23, 2022 at 5:30 PM Stefan Wahren <stefan.wahren@i2se.com> wrote:
> > >    drivers/auxdisplay: sensehat: Raspberry Pi Sense HAT display driver
> > We don't have an Ack for this display driver. Maybe we should ask Maxime
> > Ripard and Noralf Trønnes especially Pavel Machek had concerns about the
> > right subsystem?
>
> I just looked back at the comments from Pavel on v9. I had meant to
> reply to him, and I actually drafted something but it got lost in the
> shuffle and I never sent it (also my bad).
>
> The driver is an fbdev in the raspberry pi tree, and that is where we
> started when upstreaming, but right from the outset we were told that
> it couldn't be an fbdev since that subsystem is being deprecated
> the replacement is the DRM subsystem

Indeed

> but (as Matthias Brugger suggests) that is probably overkill for an
> 8x8 display. Does that settle the subsystem question?

If you were aiming for an fbdev driver, a KMS driver these days would be
notably simpler.

We've added simpler and simpler devices, such as the SSD1306 recently,
so I can't see anything wrong with using KMS either.

Maxime

On 23/08/2022 19:41, Charles Mirabile wrote:
> This patch series adds a set of drivers for operating the Sense HAT
> peripheral device. This board is an add on for the Raspberry Pi that is
> designed to connect using the GPIO connector and communicate via I2C.
> 
> It features:
> 	- a joystick
> 	- an 8x8 RGB LED matrix display
> 	- a whole bunch of environmental sensors with their own drivers
> 	  (those are already in upstream Linux)
> 
> This is a refactor of the work of Serge Schneider, the author of a
> version of this driver that is currently in the Raspberry Pi downstream
> kernel. We modified his code to make it suitable for upstream Linux.
> 
> A couple of tests are available for the driver in the test folder in
> this repo: https://github.com/underground-software/sensehat.git
> 	- sensehat_joystick_test logs the input events from the
> 	  joystick to the console
> 	- sensehat_display_test displays various solid colors on
> 	  the LED panel.
> 	- full_sensehat_test displays a single lit cell that can be
> 	  moved with the joystick. Pressing the joystick ends the
> 	  program.
> 
> For more information about the Sense HAT, visit:
> https://www.raspberrypi.org/products/sense-hat/
> 
> Changes since v9:
> 	- The driver for the sensehat joystick was accepted upstream so
> 	  it is no longer included in this patchset.
> 	- Some of the people involved in the development of earlier
> 	  versions of these patches are no longer involved in the project
> 	  so they have been removed from the list of maintainers and the
> 	  patches.

I'd would expect the patches to honor their involvement in the past, but if they 
are fine from being dropped, I will not object...

Regards,
Matthias

> 	- The code is completely unchanged and so based on the discussions
> 	  from version 9 of the patches, it should be ready to be merged.
> 
> My appologies for the long delay between v9 and v10 especially given the
> fact that there are no changes to the code. We appreciate the maintainers'
> patience and guidance throughout all 10 versions of this sensehat patchset,
> and we are excited for this driver to hopefully be fully accepted.
> 
> Best - Charlie
> 
> Co-developed-by: Joel Slebodnick <jslebodn@redhat.com>
> Signed-off-by: Joel Slebodnick <jslebodn@redhat.com>
> Co-developed-by: Joel Savitz <jsavitz@redhat.com>
> Signed-off-by: Joel Savitz <jsavitz@redhat.com>
> Signed-off-by: Charles Mirabile <cmirabil@redhat.com>
> 
> Charles Mirabile (5):
>    drivers/mfd: sensehat: Add Raspberry Pi Sense HAT to simple_mfd_i2c
>    drivers/auxdisplay: sensehat: Raspberry Pi Sense HAT display driver
>    dt-bindings: mfd: sensehat: Add Raspberry Pi Sense HAT schema
>    MAINTAINERS: Add sensehat driver authors to MAINTAINERS
>    DO NOT MERGE: full sensehat device tree overlay for raspberry pi 4
> 
>   .../raspberrypi,sensehat-display.yaml         |  26 +++
>   .../input/raspberrypi,sensehat-joystick.yaml  |  31 +++
>   .../bindings/mfd/raspberrypi,sensehat.yaml    |  57 +++++
>   MAINTAINERS                                   |  10 +
>   drivers/auxdisplay/Kconfig                    |   8 +
>   drivers/auxdisplay/Makefile                   |   1 +
>   drivers/auxdisplay/sensehat-display.c         | 205 ++++++++++++++++++
>   drivers/mfd/simple-mfd-i2c.c                  |   1 +
>   sensehat.dtbs                                 |  52 +++++
>   9 files changed, 391 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/auxdisplay/raspberrypi,sensehat-display.yaml
>   create mode 100644 Documentation/devicetree/bindings/input/raspberrypi,sensehat-joystick.yaml
>   create mode 100644 Documentation/devicetree/bindings/mfd/raspberrypi,sensehat.yaml
>   create mode 100644 drivers/auxdisplay/sensehat-display.c
>   create mode 100644 sensehat.dtbs
>

On Wed, Aug 24, 2022 at 7:28 AM Matthias Brugger <mbrugger@suse.com> wrote:
>
>
>
> On 23/08/2022 19:41, Charles Mirabile wrote:
> > This patch series adds a set of drivers for operating the Sense HAT
> > peripheral device. This board is an add on for the Raspberry Pi that is
> > designed to connect using the GPIO connector and communicate via I2C.
> >
> > It features:
> >       - a joystick
> >       - an 8x8 RGB LED matrix display
> >       - a whole bunch of environmental sensors with their own drivers
> >         (those are already in upstream Linux)
> >
> > This is a refactor of the work of Serge Schneider, the author of a
> > version of this driver that is currently in the Raspberry Pi downstream
> > kernel. We modified his code to make it suitable for upstream Linux.
> >
> > A couple of tests are available for the driver in the test folder in
> > this repo: https://github.com/underground-software/sensehat.git
> >       - sensehat_joystick_test logs the input events from the
> >         joystick to the console
> >       - sensehat_display_test displays various solid colors on
> >         the LED panel.
> >       - full_sensehat_test displays a single lit cell that can be
> >         moved with the joystick. Pressing the joystick ends the
> >         program.
> >
> > For more information about the Sense HAT, visit:
> > https://www.raspberrypi.org/products/sense-hat/
> >
> > Changes since v9:
> >       - The driver for the sensehat joystick was accepted upstream so
> >         it is no longer included in this patchset.
> >       - Some of the people involved in the development of earlier
> >         versions of these patches are no longer involved in the project
> >         so they have been removed from the list of maintainers and the
> >         patches.
>
> I'd would expect the patches to honor their involvement in the past, but if they
> are fine from being dropped, I will not object...
Since I am sending a v11 I will try to get in touch with them and see
if they want
to be involved and find proper emails (they no longer work at Red Hat).
I will also look into how much / to which parts of the code they
contributed and see
where it is appropriate to include them if they do want to be
included. I should have
done this before sending v10, but I didn't have the proper emails and
wasn't sure
about the correct etiquette.
>
> Regards,
> Matthias
>
> >       - The code is completely unchanged and so based on the discussions
> >         from version 9 of the patches, it should be ready to be merged.
> >
> > My appologies for the long delay between v9 and v10 especially given the
> > fact that there are no changes to the code. We appreciate the maintainers'
> > patience and guidance throughout all 10 versions of this sensehat patchset,
> > and we are excited for this driver to hopefully be fully accepted.
> >
> > Best - Charlie
> >
> > Co-developed-by: Joel Slebodnick <jslebodn@redhat.com>
> > Signed-off-by: Joel Slebodnick <jslebodn@redhat.com>
> > Co-developed-by: Joel Savitz <jsavitz@redhat.com>
> > Signed-off-by: Joel Savitz <jsavitz@redhat.com>
> > Signed-off-by: Charles Mirabile <cmirabil@redhat.com>
> >
> > Charles Mirabile (5):
> >    drivers/mfd: sensehat: Add Raspberry Pi Sense HAT to simple_mfd_i2c
> >    drivers/auxdisplay: sensehat: Raspberry Pi Sense HAT display driver
> >    dt-bindings: mfd: sensehat: Add Raspberry Pi Sense HAT schema
> >    MAINTAINERS: Add sensehat driver authors to MAINTAINERS
> >    DO NOT MERGE: full sensehat device tree overlay for raspberry pi 4
> >
> >   .../raspberrypi,sensehat-display.yaml         |  26 +++
> >   .../input/raspberrypi,sensehat-joystick.yaml  |  31 +++
> >   .../bindings/mfd/raspberrypi,sensehat.yaml    |  57 +++++
> >   MAINTAINERS                                   |  10 +
> >   drivers/auxdisplay/Kconfig                    |   8 +
> >   drivers/auxdisplay/Makefile                   |   1 +
> >   drivers/auxdisplay/sensehat-display.c         | 205 ++++++++++++++++++
> >   drivers/mfd/simple-mfd-i2c.c                  |   1 +
> >   sensehat.dtbs                                 |  52 +++++
> >   9 files changed, 391 insertions(+)
> >   create mode 100644 Documentation/devicetree/bindings/auxdisplay/raspberrypi,sensehat-display.yaml
> >   create mode 100644 Documentation/devicetree/bindings/input/raspberrypi,sensehat-joystick.yaml
> >   create mode 100644 Documentation/devicetree/bindings/mfd/raspberrypi,sensehat.yaml
> >   create mode 100644 drivers/auxdisplay/sensehat-display.c
> >   create mode 100644 sensehat.dtbs
> >
>

On Tue, Aug 23, 2022 at 01:41:53PM -0400, Charles Mirabile wrote:
> This patch series adds a set of drivers for operating the Sense HAT
> peripheral device. This board is an add on for the Raspberry Pi that is
> designed to connect using the GPIO connector and communicate via I2C.
> 

Better say "Add a set of drivers for Sense HAT peripheral add-on device
for Raspberry Pi. The device is connected using GPIO connector and
communicate via I2C".

> This is a refactor of the work of Serge Schneider, the author of a
> version of this driver that is currently in the Raspberry Pi downstream
> kernel. We modified his code to make it suitable for upstream Linux.
> 

s/We modified his code/The driver code is modified/

Also, I see that description of individual patches is written in
descriptive mood. However, it is preferable to write in imperative
instead (see "Describe your changes" in
Documentation/process/submitting-patches.rst). The specific suggested
wordings are in reply to each patch.

Thanks.

-- 
An old man doll... just what I always wanted! - Clara

On 06/09/2022 04:21, Bagas Sanjaya wrote:
> On Tue, Aug 23, 2022 at 01:41:53PM -0400, Charles Mirabile wrote:
>> This patch series adds a set of drivers for operating the Sense HAT
>> peripheral device. This board is an add on for the Raspberry Pi that is
>> designed to connect using the GPIO connector and communicate via I2C.
>>
> 
> Better say "Add a set of drivers for Sense HAT peripheral add-on device
> for Raspberry Pi. The device is connected using GPIO connector and
> communicate via I2C".
> 

The cover letter won't be part of the git log, so as long as it's understandable 
it's OK. It makes no sense to review the wording...

Regards,
Matthias

>> This is a refactor of the work of Serge Schneider, the author of a
>> version of this driver that is currently in the Raspberry Pi downstream
>> kernel. We modified his code to make it suitable for upstream Linux.
>>
> 
> s/We modified his code/The driver code is modified/
> 
> Also, I see that description of individual patches is written in
> descriptive mood. However, it is preferable to write in imperative
> instead (see "Describe your changes" in
> Documentation/process/submitting-patches.rst). The specific suggested
> wordings are in reply to each patch.
> 
> Thanks.
>