Received-SPF: pass (google.com: domain of linux-kernel+bounces-82496-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Precedence: bulk
MIME-Version: 1.0
References: <20240225213423.690561-1-chris.packham@alliedtelesis.co.nz>
 <CAHp75Vc9OBtxdKSmk9Uu9G3j+mfN8+9prTEVx3LyUcdBYFEqwg@mail.gmail.com> <584172eb-4eda-40d1-9ee5-99d0ef944481@alliedtelesis.co.nz>
In-Reply-To: <584172eb-4eda-40d1-9ee5-99d0ef944481@alliedtelesis.co.nz>
From: Andy Shevchenko <andy.shevchenko@gmail.com>
Date: Tue, 27 Feb 2024 02:58:23 +0200
Message-ID: <CAHp75Vc3NFULXByhAv=1bq0aPsbH+_zVypQwaDGNrtafj+xGaw@mail.gmail.com>
Subject: Re: [PATCH 0/3] auxdisplay: 7 segment LED display
To: Chris Packham <Chris.Packham@alliedtelesis.co.nz>
Cc: "ojeda@kernel.org" <ojeda@kernel.org>, "robh+dt@kernel.org" <robh+dt@kernel.org>, 
	"krzysztof.kozlowski+dt@linaro.org" <krzysztof.kozlowski+dt@linaro.org>, 
	"conor+dt@kernel.org" <conor+dt@kernel.org>, "andrew@lunn.ch" <andrew@lunn.ch>, 
	"gregory.clement@bootlin.com" <gregory.clement@bootlin.com>, 
	"sebastian.hesselbarth@gmail.com" <sebastian.hesselbarth@gmail.com>, 
	"geert@linux-m68k.org" <geert@linux-m68k.org>, "pavel@ucw.cz" <pavel@ucw.cz>, "lee@kernel.org" <lee@kernel.org>, 
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>, 
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, 
	"linux-arm-kernel@lists.infradead.org" <linux-arm-kernel@lists.infradead.org>, 
	"linux-leds@vger.kernel.org" <linux-leds@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Tue, Feb 27, 2024 at 2:52=E2=80=AFAM Chris Packham
<Chris.Packham@alliedtelesis.co.nz> wrote:
> On 26/02/24 15:23, Andy Shevchenko wrote:
> > On Sun, Feb 25, 2024 at 11:34=E2=80=AFPM Chris Packham
> > <chris.packham@alliedtelesis.co.nz> wrote:
> >> This series adds a driver for a 7 segment LED display.
> >>
> >> I'd like to get some feedback on how this could be extended to support=
 >1
> >> character. The driver as presented is sufficient for my hardware which=
 only has
> >> a single character display but I can see that for this to be generical=
ly useful
> >> supporting more characters would be desireable.
> >>
> >> Earlier I posted an idea that the characters could be represended by
> >> sub-nodes[1] but there doesn't seem to be a way of having that and kee=
ping the
> >> convenience of using devm_gpiod_get_array() (unless I've missed someth=
ing).
> > It seems you didn't know that the tree for auxdisplay has been changed.
> > Can you rebase your stuff on top of
> > https://scanmail.trustwave.com/?c=3D20988&d=3Dvfbb5fnU59kvIREfdD-21Pab3=
0bpMpuTM2Ipv28now&u=3Dhttps%3a%2f%2fgit%2ekernel%2eorg%2fpub%2fscm%2flinux%=
2fkernel%2fgit%2fandy%2flinux-auxdisplay%2egit%2flog%2f%3fh%3dfor-next%3f
> > It will reduce your code base by ~50%.
> >
> > WRT subnodes, you can go with device_for_each_child_node() and
> > retrieve gpio array per digit. It means you will have an array of
> > arrays of GPIOs.
>
> So would the following work?
>
>      count =3D device_get_child_node_count(dev);
>      struct gpio_descs **chars  =3D devm_kzalloc(dev, sizeof(*chars) *
> count, GFP_KERNEL);
>
>      i =3D 0;
>      device_for_each_child_node(dev, child) {
>          chars[i] =3D devm_gpiod_get_array(dev, "segment", GPIOD_OUT_LOW)=
;

I see what you meant earlier.
This should be devm_fwnode_gpiod_get_index(), but we lack the _array
variant of it...

Dunno what to do, maybe adding the _array variant is the good move,
maybe something else.

>      }
>
> I haven't used the child. The devm_gpiod_get_array() will be looking at
> the fwnode of the parent.


--=20
With Best Regards,
Andy Shevchenko