Re: [RFC PATCH] drivers: Raspbery Pi Sense Hat

From: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
To: Joel Savitz <jsavitz@redhat.com>, linux-rpi-kernel@lists.infradead.org
Cc: Joel Savitz <joelsavitz@gmail.com>,
	bcm-kernel-feedback-list@broadcom.com,
	 linux-arm-kernel@lists.infradead.org,
	fedora-rpi@googlegroups.com,  serge@raspberrypi.org
Subject: Re: [RFC PATCH] drivers: Raspbery Pi Sense Hat
Date: Thu, 08 Apr 2021 18:16:09 +0200	[thread overview]
Message-ID: <544611301cff5db027582f531880c41a87569116.camel@suse.de> (raw)
In-Reply-To: <20210408000353.3571132-1-jsavitz@redhat.com>

[-- Attachment #1.1: Type: text/plain, Size: 1495 bytes --]

Hi Joel, thanks for having a look at this.

On Wed, 2021-04-07 at 20:03 -0400, Joel Savitz wrote:
> From: Joel Savitz <joelsavitz@gmail.com>
> 
> This is a forward port of the Rapsberry Pi Sense Hat driver from the
> downstream Raspberry Pi kernel tree.
> 
> I have slightly consolidated things and done some quick testing of this
> driver on Fedora (which comes with the device tree blobs), and I want to
> do a bit more testing before submiting the driver for inclusion in
> mainline Linux.
> 
> I would like to get some feedback from the community on what may need to
> be improved, fixed, or changed as early as possible, so this is a request
> for comments.
> 
> Signed-off-by: Joel Savitz <joelsavitz@gmail.com>
> ---

Some random comments:

 - Can you share more info on the HW, is this a uC on the other side? etc...
 - A driver that depends on DT should have bindings.
 - Please have a look at a modern MFD device handles things. New APIs appeared.
   See for example the 'atc260x' driver available in linux-next.
 - Try to use regmap so as not having to export functions across modules.
 - No need to explicitly schedule the interrupt in the joystick driver. New IRQ
   APIs can handle that. See devm_request_threaded_irq().
 - I don't think adding new FB drivers is possible, the whole subsystem is in
   the process of being deprecated. I believe there is an equivalent framework
   in DRM to cater for these simple FB devices.

Regards,
Nicolas

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel