mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Nicolas Saenz Julienne <nsaenzju@redhat.com>
To: Charles Mirabile <cmirabil@redhat.com>, linux-kernel@vger.kernel.org
Cc: Serge Schneider <serge@raspberrypi.org>,
	Stefan Wahren <stefan.wahren@i2se.com>,
	Mattias Brugger <mbrugger@suse.com>,
	linux-rpi-kernel@lists.infradead.org,
	linux-arm-kernel@lists.infradead.org,
	fedora-rpi@googlegroups.com, Mwesigwa Guma <mguma@redhat.com>,
	Joel Savitz <jsavitz@redhat.com>
Subject: Re: [PATCH V5 0/6] Raspberry Pi Sense HAT driver
Date: Mon, 13 Dec 2021 12:29:52 +0100	[thread overview]
Message-ID: <ff321353fa27a156a0eab9cc9674dfad760f22a8.camel@redhat.com> (raw)
In-Reply-To: <20211210221033.912430-1-cmirabil@redhat.com>

Hi Charles, Thanks for iterating on this.

On Fri, 2021-12-10 at 17:10 -0500, Charles Mirabile wrote:
> Suggestions from v4 not addressed yet:
> 	- Use mfd_cells for the joystick and display platform devices
> 	  (as suggested by Matthias Brugger) or remove sensehat-core.c
> 	  entirely and use simple-mfd-i2c instead (as suggested by
> 	  Nicolás Sáenz Julienne).
> 	  	- while these suggestions might make great improvements
> 	  	  for a version two of the driver they would require a
> 	  	  large rewrite to move all of the complexity in core
> 	  	  elsewhere. While certainly possible and something
> 	  	  we want to iterate on and achieve eventually, we
> 	  	  think saving it for down the road makes sense at this
> 	  	  time.

Sorry for insisting, but I'm not convinced by this approach. The way you design
your driver affects its devicetree bindings, once we settle on those any
upgrade will have to be backwards compatible (i.e. an old DT would have to work
with the new implementation). This makes changing this further down the road
unlikely.

In terms of complexity, most of it is there due to not using the APIs and
facilities the mfd subsystem provides. See my comments in the driver.

Regards,

-- 
Nicolás Sáenz


      parent reply	other threads:[~2021-12-13 11:29 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-10 22:10 Charles Mirabile
2021-12-10 22:10 ` [PATCH V5 1/6] drivers/mfd: sensehat: Raspberry Pi Sense HAT core driver Charles Mirabile
2021-12-11 20:53   ` Thomas Weißschuh
2021-12-12 11:43   ` Stefan Wahren
2021-12-13 11:49   ` Nicolas Saenz Julienne
2021-12-10 22:10 ` [PATCH V5 2/6] drivers/input/joystick: sensehat: Raspberry Pi Sense HAT joystick driver Charles Mirabile
2021-12-10 23:17   ` Dmitry Torokhov
2021-12-10 22:10 ` [PATCH V5 3/6] drivers/auxdisplay: sensehat: Raspberry Pi Sense HAT display driver Charles Mirabile
2021-12-11 18:38   ` Miguel Ojeda
2021-12-10 22:10 ` [PATCH V5 4/6] dt-bindings: mfd: sensehat: Add Raspberry Pi Sense HAT schema Charles Mirabile
2021-12-11 19:59   ` Rob Herring
2021-12-13 17:09     ` Rob Herring
2021-12-10 22:10 ` [PATCH V5 5/6] MAINTAINERS: Add sensehat driver authors to MAINTAINERS Charles Mirabile
2021-12-10 22:10 ` [PATCH V5 6/6] DO NOT MERGE: full sensehat device tree overlay for raspberry pi 4 Charles Mirabile
2021-12-13 11:29 ` Nicolas Saenz Julienne [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ff321353fa27a156a0eab9cc9674dfad760f22a8.camel@redhat.com \
    --to=nsaenzju@redhat.com \
    --cc=cmirabil@redhat.com \
    --cc=fedora-rpi@googlegroups.com \
    --cc=jsavitz@redhat.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rpi-kernel@lists.infradead.org \
    --cc=mbrugger@suse.com \
    --cc=mguma@redhat.com \
    --cc=serge@raspberrypi.org \
    --cc=stefan.wahren@i2se.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®