From: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
To: Lee Jones <lee.jones@linaro.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
linuxarm@huawei.com, mauro.chehab@huawei.com,
Rob Herring <robh+dt@kernel.org>,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-staging@lists.linux.dev
Subject: Re: [PATCH RESEND v6 6/8] mfd: hi6421-spmi-pmic: move driver from staging
Date: Fri, 25 Jun 2021 13:13:56 +0200 [thread overview]
Message-ID: <20210625131356.55ffd067@coco.lan> (raw)
In-Reply-To: <YNSRwIMr8+m9Sxk3@dell>
Em Thu, 24 Jun 2021 15:08:00 +0100
Lee Jones <lee.jones@linaro.org> escreveu:
> > > > +/*
> > > > + * The IRQs are mapped as:
> > > > + *
> > > > + * ====================== ============= ============ =====
> > > > + * IRQ MASK REGISTER IRQ REGISTER BIT
> > > > + * ====================== ============= ============ =====
> > > > + * OTMP 0x0202 0x212 bit 0
> > > > + * VBUS_CONNECT 0x0202 0x212 bit 1
> > > > + * VBUS_DISCONNECT 0x0202 0x212 bit 2
> > > > + * ALARMON_R 0x0202 0x212 bit 3
> > > > + * HOLD_6S 0x0202 0x212 bit 4
> > > > + * HOLD_1S 0x0202 0x212 bit 5
> > > > + * POWERKEY_UP 0x0202 0x212 bit 6
> > > > + * POWERKEY_DOWN 0x0202 0x212 bit 7
> > > > + *
> > > > + * OCP_SCP_R 0x0203 0x213 bit 0
> > > > + * COUL_R 0x0203 0x213 bit 1
> > > > + * SIM0_HPD_R 0x0203 0x213 bit 2
> > > > + * SIM0_HPD_F 0x0203 0x213 bit 3
> > > > + * SIM1_HPD_R 0x0203 0x213 bit 4
> > > > + * SIM1_HPD_F 0x0203 0x213 bit 5
> > > > + * ====================== ============= ============ =====
> > > > + *
> > > > + * Each mask register contains 8 bits. The ancillary macros below
> > > > + * convert a number from 0 to 14 into a register address and a bit mask
> > > > + */
> > > > +#define HISI_IRQ_MASK_REG(irq_data) (SOC_PMIC_IRQ_MASK_0_ADDR + \
> > > > + (irqd_to_hwirq(irq_data) / BITS_PER_BYTE))
> > > > +#define HISI_IRQ_MASK_BIT(irq_data) BIT(irqd_to_hwirq(irq_data) & (BITS_PER_BYTE - 1))
> > > > +#define HISI_8BITS_MASK GENMASK(BITS_PER_BYTE - 1, 0)
> > >
> > > Are these lines up in real code? Looks like they're not in the diff.
> >
> > Weird. The changes to use those are at patch 3/8. All the above
> > macros are used at the patch.
>
> Sorry, that made no sense - it's been a long few days!
>
> I meant to say "do these (the tabs) line up?"
Yes, they line up (and aligned with the parenthesis, in the case of
HISI_IRQ_MASK_REG).
> > > > +static const struct mfd_cell hi6421v600_devs[] = {
> > > > + { .name = "hi6421v600-regulator", },
> > > > +};
> > >
> > > Where are the other devices?
> >
> > While this is a MFD device, as it has regulators, ADC and other
> > stuff, right now, only the regulator and the IRQs are implemented.
> >
> > The IRQs are at the core of this driver, while the regulator
> > is at the separate regulator driver.
>
> The rule usually goes:
>
> Drivers don't qualify as MFDs until you register >1 device.
Do you mean that, in order for this to be accepted, should
I move the irq code to a separate driver?
> > > > + for (i = 0; i < PMIC_IRQ_LIST_MAX; i++) {
> > > > + virq = irq_create_mapping(ddata->domain, i);
> > > > + if (!virq) {
> > > > + dev_err(dev, "Failed to map H/W IRQ\n");
> > > > + return -ENOSPC;
> > >
> > > -ENOSPC doesn't seem right here.
> > >
> > > Can't find any other uses of it for irq_create_mapping() either.
> >
> > There are two drivers returning -ENOSPC:
> >
> > arch/powerpc/platforms/pseries/msi.c
> > arch/powerpc/sysdev/mpic_u3msi.c
>
> I only looked in drivers/
>
> > But others return -EIO, -EINVAL, -ENOMEM, -ENODEV, -ENXIO.
> >
> > I think that -ENODEV would fit better here.
>
> I think -ENXIO is the most common, followed by -EINVAL.
>
> This doesn't have anything to do with devices per say.
Ok. I'll change it to -ENXIO.
> > > > +static void hi6421_spmi_pmic_remove(struct spmi_device *pdev)
> > > > +{
> > > > + struct hi6421_spmi_pmic *ddata = dev_get_drvdata(&pdev->dev);
> > > > +
> > > > + free_irq(ddata->irq, ddata);
> > >
> > > No devm_* version?
> >
> > Are there a devm_* variant for gpio_to_irq()?
>
> Please refer to Dan's response.
Ok.
Thanks,
Mauro
next prev parent reply other threads:[~2021-06-25 11:14 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-24 9:01 [PATCH RESEND v6 0/8] Move Hisilicon 6421v600 SPMI and USB drivers out of staging Mauro Carvalho Chehab
2021-06-24 9:01 ` [PATCH RESEND v6 1/8] staging: phy-hi3670-usb3: do a some minor cleanups Mauro Carvalho Chehab
2021-06-24 9:01 ` [PATCH RESEND v6 2/8] staging: hisi-spmi-controller: rename spmi-channel property Mauro Carvalho Chehab
2021-06-24 9:01 ` [PATCH RESEND v6 3/8] staging: phy-hi3670-usb3: do some additional cleanups Mauro Carvalho Chehab
2021-06-24 9:01 ` [PATCH RESEND v6 4/8] phy: phy-hi3670-usb3: move driver from staging into phy Mauro Carvalho Chehab
2021-06-24 9:01 ` [PATCH RESEND v6 5/8] spmi: hisi-spmi-controller: move driver from staging Mauro Carvalho Chehab
2021-06-24 9:01 ` [PATCH RESEND v6 6/8] mfd: hi6421-spmi-pmic: " Mauro Carvalho Chehab
2021-06-24 11:33 ` Lee Jones
2021-06-24 12:36 ` Mauro Carvalho Chehab
2021-06-24 13:07 ` Dan Carpenter
2021-06-24 14:08 ` Lee Jones
2021-06-24 14:26 ` Johan Hovold
2021-06-25 11:38 ` Mauro Carvalho Chehab
2021-06-28 10:43 ` Lee Jones
2021-06-28 10:43 ` Lee Jones
2021-06-25 11:13 ` Mauro Carvalho Chehab [this message]
2021-06-24 15:26 ` Rob Herring
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=20210625131356.55ffd067@coco.lan \
--to=mchehab+huawei@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=gregkh@linuxfoundation.org \
--cc=lee.jones@linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-staging@lists.linux.dev \
--cc=linuxarm@huawei.com \
--cc=mauro.chehab@huawei.com \
--cc=robh+dt@kernel.org \
/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;
as well as URLs for NNTP newsgroup(s).