linux-kernel - Re: [PATCH v4 4/5] mfd: rn5t618: add more subdevices

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 17 Dec 2019 12:25:26 +0100
From:   Andreas Kemnade <andreas@...nade.info>
To:     Lee Jones <lee.jones@...aro.org>
Cc:     robh+dt@...nel.org, mark.rutland@....com, a.zummo@...ertech.it,
        alexandre.belloni@...tlin.com, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-rtc@...r.kernel.org,
        stefan@...er.ch, b.galvani@...il.com, phh@....me,
        letux-kernel@...nphoenux.org
Subject: Re: [PATCH v4 4/5] mfd: rn5t618: add more subdevices

On Tue, 17 Dec 2019 08:01:46 +0000
Lee Jones <lee.jones@...aro.org> wrote:

> On Mon, 16 Dec 2019, Andreas Kemnade wrote:
> 
> > On Mon, 16 Dec 2019 15:31:06 +0000
> > Lee Jones <lee.jones@...aro.org> wrote:
> >   
> > > On Wed, 11 Dec 2019, Andreas Kemnade wrote:
> > >   
> > > > The RC5T619 has a RTC which is missing in the
> > > > RN5T618. Add it as subdevice to prepare for their implementation
> > > > 
> > > > Signed-off-by: Andreas Kemnade <andreas@...nade.info>
> > > > ---
> > > > Changes in v3:
> > > > - alignment cleanup
> > > >  drivers/mfd/rn5t618.c | 16 ++++++++++++++--
> > > >  1 file changed, 14 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/drivers/mfd/rn5t618.c b/drivers/mfd/rn5t618.c
> > > > index d78eb29b94a4..18d56a732b20 100644
> > > > --- a/drivers/mfd/rn5t618.c
> > > > +++ b/drivers/mfd/rn5t618.c
> > > > @@ -22,6 +22,12 @@ static const struct mfd_cell rn5t618_cells[] = {
> > > >  	{ .name = "rn5t618-wdt" },
> > > >  };
> > > >  
> > > > +static const struct mfd_cell rc5t619_cells[] = {
> > > > +	{ .name = "rn5t618-regulator" },
> > > > +	{ .name = "rc5t619-rtc" },
> > > > +	{ .name = "rn5t618-wdt" },
> > > > +};
> > > > +
> > > >  static bool rn5t618_volatile_reg(struct device *dev, unsigned int reg)
> > > >  {
> > > >  	switch (reg) {
> > > > @@ -173,8 +179,14 @@ static int rn5t618_i2c_probe(struct i2c_client *i2c,
> > > >  		return ret;
> > > >  	}
> > > >  
> > > > -	ret = devm_mfd_add_devices(&i2c->dev, -1, rn5t618_cells,
> > > > -				   ARRAY_SIZE(rn5t618_cells), NULL, 0, NULL);
> > > > +	if (priv->variant == RC5T619)
> > > > +		ret = devm_mfd_add_devices(&i2c->dev, -1, rc5t619_cells,    
> > > 
> > > Ref: The "-1", please use this as an opportunity to use the defines.
> > >   
BTW: of course I can clean that up.

> > > > +					   ARRAY_SIZE(rc5t619_cells),
> > > > +					   NULL, 0, NULL);
> > > > +	else    
> > > 
> > > Are you sure it's not possible for 'variant' to be an unsupported
> > > device?
> > >   
> > Well, does it change the behavior for devices other than the rc5t619?
> > I do not think so. If the mfd driver is bound to unsupported devices,
> > rn5t618_of_match is wrong.  
> 
> Right, and can you catch that?
> 
Well, maybe we don't get each other. RC5T619 has an RTC.
If I understand the code right, priv->variant is set to RC5T619.
if there is compatible = "ricoh,rc5t619" in the device tree.
So in that and only in that case I have an extended subdevice list,
which includes the RTC subdevice instead of only regulator and wdt.
For everything else I do not touch it.

So now you have doubts wether the existing regulator and watchdog subdevice
drivers (which I do not touch) works with the other two variants (RN5T618 
and RN5T567) and want me to check that again as part of my work to get
RTC support for RC5T619 in? 

Regards,
Andreas

Content of type "application/pgp-signature" skipped