All of lore.kernel.org
 help / color / mirror / Atom feed
From: Shawn Guo <shawn.guo@linaro.org>
To: Florian Fainelli <f.fainelli@gmail.com>
Cc: Marc Zyngier <maz@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Maulik Shah <quic_mkshah@quicinc.com>,
	Bjorn Andersson <bjorn.andersson@linaro.org>,
	Loic Poulain <loic.poulain@linaro.org>,
	linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org,
	Claudiu Beznea <claudiu.beznea@microchip.com>,
	Neil Armstrong <narmstrong@baylibre.com>
Subject: Re: [PATCH v3 1/3] irqchip: Pass platform_device pointer to init_cb
Date: Mon, 6 Dec 2021 14:35:35 +0800	[thread overview]
Message-ID: <20211206063534.GK10105@dragon> (raw)
In-Reply-To: <b221fec0-43d0-537d-d78e-84da10a9c2d7@gmail.com>

On Thu, Dec 02, 2021 at 09:52:55AM -0800, Florian Fainelli wrote:
> On 12/2/21 4:21 AM, Shawn Guo wrote:
> > It makes sense to just pass device_node for callback in IRQCHIP_DECLARE
> > case, but not so much for IRQCHIP_PLATFORM_DRIVER one, because
> > platform_driver probe/init usually needs device pointer for various
> > purposes, e.g. resource allocation, service request, device prefixed
> > message output, etc.  Create a new callback type irqchip_init_cb_t which
> > takes platform_device pointer as parameter, and update the existing
> > IRQCHIP_PLATFORM_DRIVER users accordingly.
> > 
> > Cc: Florian Fainelli <f.fainelli@gmail.com>
> > Cc: Claudiu Beznea <claudiu.beznea@microchip.com>
> > Cc: Neil Armstrong <narmstrong@baylibre.com>
> > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> 
> Could you copy all recipients on all 3 patches plus your cover letter
> next time so we have the full context? Thanks!
> 
> [snip]
> 
> >  
> > -static int __init bcm7120_l2_intc_probe_7120(struct device_node *dn,
> > +static int __init bcm7120_l2_intc_probe_7120(struct platform_device *pdev,
> >  					     struct device_node *parent)
> >  {
> > -	return bcm7120_l2_intc_probe(dn, parent, bcm7120_l2_intc_iomap_7120,
> > +	return bcm7120_l2_intc_probe(pdev->dev.of_node, parent,
> > +				     bcm7120_l2_intc_iomap_7120,
> >  				     "BCM7120 L2");
> 
> If you look further into that driver, you will see that we do something
> like this in bcm7120_l2_intc_probe:
> 
>           pdev = of_find_device_by_node(dn);
>           if (!pdev) {
>                   ret = -ENODEV;
>                   goto out_free_data;
>           }
> 
> which would be completely superfluous now that we pass a platform_device
> directly. Can you rework your patch so as to eliminate that
> of_find_device_by_ndoe() (and the companion put_device call)?

Firstly, I do not see any companion put_device call in the driver.
Secondly, the existing code seems to have some problem in the "out"
order.  The out_unmap should go before out_free_l1_data, right?

@@ -329,13 +323,13 @@ static int __init bcm7120_l2_intc_probe(struct device_node *dn,
 
 out_free_domain:
        irq_domain_remove(data->domain);
-out_free_l1_data:
-       kfree(data->l1_data);
 out_unmap:
        for (idx = 0; idx < MAX_MAPPINGS; idx++) {
                if (data->map_base[idx])
                        iounmap(data->map_base[idx]);
        }
+out_free_l1_data:
+       kfree(data->l1_data);
 out_free_data:
        kfree(data);
        return ret;

Shawn

  parent reply	other threads:[~2021-12-06  6:35 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-02 12:21 [PATCH v3 0/3] Add Qualcomm MPM irqchip driver support Shawn Guo
2021-12-02 12:21 ` [PATCH v3 1/3] irqchip: Pass platform_device pointer to init_cb Shawn Guo
2021-12-02 17:52   ` Florian Fainelli
2021-12-02 19:10     ` Marc Zyngier
2021-12-02 21:44       ` Florian Fainelli
2021-12-06  6:40       ` Shawn Guo
2021-12-06  8:29         ` Marc Zyngier
2021-12-06  6:35     ` Shawn Guo [this message]
2021-12-02 12:21 ` [PATCH v3 2/3] dt-bindings: interrupt-controller: Add Qualcomm MPM support Shawn Guo
2021-12-02 12:21 ` [PATCH v3 3/3] irqchip: Add Qualcomm MPM controller driver Shawn Guo
2021-12-03  0:00   ` kernel test robot
2021-12-03  0:00     ` kernel test robot
2021-12-06  9:46   ` Marc Zyngier
2021-12-06 13:15     ` Shawn Guo
2021-12-06 13:48       ` Marc Zyngier
2021-12-07  9:48         ` Shawn Guo
2021-12-07 10:16           ` Marc Zyngier
2021-12-08 10:04             ` Shawn Guo

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=20211206063534.GK10105@dragon \
    --to=shawn.guo@linaro.org \
    --cc=bjorn.andersson@linaro.org \
    --cc=claudiu.beznea@microchip.com \
    --cc=f.fainelli@gmail.com \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=loic.poulain@linaro.org \
    --cc=maz@kernel.org \
    --cc=narmstrong@baylibre.com \
    --cc=quic_mkshah@quicinc.com \
    --cc=tglx@linutronix.de \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.