PCI: pcie-rcar: Cache PHY init function pointer

Commit Message

Marek Vasut April 26, 2020, 12:31 p.m. UTC

From: Marek Vasut <[e-mail]>

The PHY initialization function pointer does not change during the
lifetime of the driver instance, it is therefore sufficient to get
the pointer in .probe(), cache it in driver private data, and just
call the function through the cached pointer in .resume().

Signed-off-by: Marek Vasut <[e-mail]>
Cc: Bjorn Helgaas <[e-mail]>
Cc: Lorenzo Pieralisi <[e-mail]>
Cc: Geert Uytterhoeven <[e-mail]>
Cc: Wolfram Sang <[e-mail]>
Cc: [e-mail]
---
NOTE: Based on git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/pci.git
      branch pci/rcar
NOTE: The driver tag is now 'pcie-rcar' to distinguish it from pci-rcar-gen2.c
---
 drivers/pci/controller/pcie-rcar.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

Comments

Geert Uytterhoeven April 27, 2020, 7:19 a.m. UTC | #1

On Sun, Apr 26, 2020 at 2:31 PM <[e-mail]> wrote:
> From: Marek Vasut <[e-mail]>
>
> The PHY initialization function pointer does not change during the
> lifetime of the driver instance, it is therefore sufficient to get
> the pointer in .probe(), cache it in driver private data, and just
> call the function through the cached pointer in .resume().
>
> Signed-off-by: Marek Vasut <[e-mail]>

Reviewed-by: Geert Uytterhoeven <[e-mail]>

Gr{oetje,eeting}s,

                        Geert

Lorenzo Pieralisi April 28, 2020, 8:32 a.m. UTC | #2

On Sun, Apr 26, 2020 at 02:31:47PM +0200, [e-mail] wrote:
> From: Marek Vasut <[e-mail]>
> 
> The PHY initialization function pointer does not change during the
> lifetime of the driver instance, it is therefore sufficient to get
> the pointer in .probe(), cache it in driver private data, and just
> call the function through the cached pointer in .resume().
> 
> Signed-off-by: Marek Vasut <[e-mail]>
> Cc: Bjorn Helgaas <[e-mail]>
> Cc: Lorenzo Pieralisi <[e-mail]>
> Cc: Geert Uytterhoeven <[e-mail]>
> Cc: Wolfram Sang <[e-mail]>
> Cc: [e-mail]
> ---
> NOTE: Based on git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/pci.git
>       branch pci/rcar
> NOTE: The driver tag is now 'pcie-rcar' to distinguish it from pci-rcar-gen2.c
> ---
>  drivers/pci/controller/pcie-rcar.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)

Squashed in https://patchwork.kernel.org/patch/11438665

Do you want me to rename the $SUBJECT (and the branch name while at it)
in the patches in my pci/rcar branch ("PCI: pcie-rcar: ...") to start
the commit subject tag renaming from this cycle (and in the interim you
send a rename for the drivers files ?)

Lorenzo

> diff --git a/drivers/pci/controller/pcie-rcar.c b/drivers/pci/controller/pcie-rcar.c
> index 1a0e74cad9bb..59e55f56e386 100644
> --- a/drivers/pci/controller/pcie-rcar.c
> +++ b/drivers/pci/controller/pcie-rcar.c
> @@ -153,6 +153,7 @@ struct rcar_pcie {
>  	int			root_bus_nr;
>  	struct clk		*bus_clk;
>  	struct			rcar_msi msi;
> +	int			(*phy_init_fn)(struct rcar_pcie *pcie);
>  };
>  
>  static void rcar_pci_write_reg(struct rcar_pcie *pcie, u32 val,
> @@ -1147,7 +1148,6 @@ static int rcar_pcie_probe(struct platform_device *pdev)
>  	struct rcar_pcie *pcie;
>  	u32 data;
>  	int err;
> -	int (*phy_init_fn)(struct rcar_pcie *);
>  	struct pci_host_bridge *bridge;
>  
>  	bridge = pci_alloc_host_bridge(sizeof(*pcie));
> @@ -1187,8 +1187,8 @@ static int rcar_pcie_probe(struct platform_device *pdev)
>  	if (err)
>  		goto err_clk_disable;
>  
> -	phy_init_fn = of_device_get_match_data(dev);
> -	err = phy_init_fn(pcie);
> +	pcie->phy_init_fn = of_device_get_match_data(dev);
> +	err = pcie->phy_init_fn(pcie);
>  	if (err) {
>  		dev_err(dev, "failed to init PCIe PHY\n");
>  		goto err_clk_disable;
> @@ -1253,7 +1253,6 @@ static int rcar_pcie_probe(struct platform_device *pdev)
>  static int __maybe_unused rcar_pcie_resume(struct device *dev)
>  {
>  	struct rcar_pcie *pcie = dev_get_drvdata(dev);
> -	int (*hw_init_fn)(struct rcar_pcie *);
>  	unsigned int data;
>  	int err;
>  
> @@ -1262,8 +1261,7 @@ static int __maybe_unused rcar_pcie_resume(struct device *dev)
>  		return 0;
>  
>  	/* Failure to get a link might just be that no cards are inserted */
> -	hw_init_fn = of_device_get_match_data(dev);
> -	err = hw_init_fn(pcie);
> +	err = pcie->phy_init_fn(pcie);
>  	if (err) {
>  		dev_info(dev, "PCIe link down\n");
>  		return 0;
> -- 
> 2.25.1
>

Marek Vasut May 1, 2020, 8:42 p.m. UTC | #3

On 4/28/20 10:32 AM, Lorenzo Pieralisi wrote:
> On Sun, Apr 26, 2020 at 02:31:47PM +0200, [e-mail] wrote:
>> From: Marek Vasut <[e-mail]>
>>
>> The PHY initialization function pointer does not change during the
>> lifetime of the driver instance, it is therefore sufficient to get
>> the pointer in .probe(), cache it in driver private data, and just
>> call the function through the cached pointer in .resume().
>>
>> Signed-off-by: Marek Vasut <[e-mail]>
>> Cc: Bjorn Helgaas <[e-mail]>
>> Cc: Lorenzo Pieralisi <[e-mail]>
>> Cc: Geert Uytterhoeven <[e-mail]>
>> Cc: Wolfram Sang <[e-mail]>
>> Cc: [e-mail]
>> ---
>> NOTE: Based on git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/pci.git
>>       branch pci/rcar
>> NOTE: The driver tag is now 'pcie-rcar' to distinguish it from pci-rcar-gen2.c
>> ---
>>  drivers/pci/controller/pcie-rcar.c | 10 ++++------
>>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> Squashed in https://patchwork.kernel.org/patch/11438665

Thanks

> Do you want me to rename the $SUBJECT (and the branch name while at it)
> in the patches in my pci/rcar branch ("PCI: pcie-rcar: ...") to start
> the commit subject tag renaming from this cycle (and in the interim you
> send a rename for the drivers files ?)

I don't really have a particular preference either way. I can keep
marking the drivers with pcie-rcar and pci-rcar tags if that helps
discern them.

Bjorn Helgaas May 1, 2020, 9:52 p.m. UTC | #4

On Tue, Apr 28, 2020 at 09:32:31AM +0100, Lorenzo Pieralisi wrote:
> On Sun, Apr 26, 2020 at 02:31:47PM +0200, [e-mail] wrote:
> > From: Marek Vasut <[e-mail]>
> > 
> > The PHY initialization function pointer does not change during the
> > lifetime of the driver instance, it is therefore sufficient to get
> > the pointer in .probe(), cache it in driver private data, and just
> > call the function through the cached pointer in .resume().
> > 
> > Signed-off-by: Marek Vasut <[e-mail]>
> > Cc: Bjorn Helgaas <[e-mail]>
> > Cc: Lorenzo Pieralisi <[e-mail]>
> > Cc: Geert Uytterhoeven <[e-mail]>
> > Cc: Wolfram Sang <[e-mail]>
> > Cc: [e-mail]
> > ---
> > NOTE: Based on git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/pci.git
> >       branch pci/rcar
> > NOTE: The driver tag is now 'pcie-rcar' to distinguish it from pci-rcar-gen2.c
> > ---
> >  drivers/pci/controller/pcie-rcar.c | 10 ++++------
> >  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> Squashed in https://patchwork.kernel.org/patch/11438665
> 
> Do you want me to rename the $SUBJECT (and the branch name while at it)
> in the patches in my pci/rcar branch ("PCI: pcie-rcar: ...") to start
> the commit subject tag renaming from this cycle (and in the interim you
> send a rename for the drivers files ?)

My vote is a tag of "rcar" for the pcie-rcar driver because almost all
new drivers are PCIe, and none of the others use "pcie-" in the tag.

For pci-rcar-gen2.c, we could use "rcar-gen2" (already used by the
last 5 commits, last touched over two years ago).  It's slightly
confusing to use "gen2" to refer to some internal R-Car thing instead
of PCIe Gen 2, so we could use something like "rcar-pci", but I'm not
sure it's worth it.

Bjorn

Lorenzo Pieralisi May 5, 2020, 6:02 p.m. UTC | #5

On Fri, May 01, 2020 at 10:42:06PM +0200, Marek Vasut wrote:
> On 4/28/20 10:32 AM, Lorenzo Pieralisi wrote:
> > On Sun, Apr 26, 2020 at 02:31:47PM +0200, [e-mail] wrote:
> >> From: Marek Vasut <[e-mail]>
> >>
> >> The PHY initialization function pointer does not change during the
> >> lifetime of the driver instance, it is therefore sufficient to get
> >> the pointer in .probe(), cache it in driver private data, and just
> >> call the function through the cached pointer in .resume().
> >>
> >> Signed-off-by: Marek Vasut <[e-mail]>
> >> Cc: Bjorn Helgaas <[e-mail]>
> >> Cc: Lorenzo Pieralisi <[e-mail]>
> >> Cc: Geert Uytterhoeven <[e-mail]>
> >> Cc: Wolfram Sang <[e-mail]>
> >> Cc: [e-mail]
> >> ---
> >> NOTE: Based on git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/pci.git
> >>       branch pci/rcar
> >> NOTE: The driver tag is now 'pcie-rcar' to distinguish it from pci-rcar-gen2.c
> >> ---
> >>  drivers/pci/controller/pcie-rcar.c | 10 ++++------
> >>  1 file changed, 4 insertions(+), 6 deletions(-)
> > 
> > Squashed in https://patchwork.kernel.org/patch/11438665
> 
> Thanks
> 
> > Do you want me to rename the $SUBJECT (and the branch name while at it)
> > in the patches in my pci/rcar branch ("PCI: pcie-rcar: ...") to start
> > the commit subject tag renaming from this cycle (and in the interim you
> > send a rename for the drivers files ?)
> 
> I don't really have a particular preference either way. I can keep
> marking the drivers with pcie-rcar and pci-rcar tags if that helps
> discern them.

So:

- "rcar" for the PCIe driver
- "rcar-pci" or "rcar-legacy" for the pci-rcar-gen2.c (preference ?
  there is no urgency, no commit queued to rename, it is for future
  code)

Are we OK with that ? If yes I will rewrite the commits subjects
and push out an updated pci/rcar branch.

...DT bindings commit subjects - should I change their tag subject
too ?

Lorenzo

Marek Vasut May 5, 2020, 6:35 p.m. UTC | #6

On 5/5/20 8:02 PM, Lorenzo Pieralisi wrote:
> On Fri, May 01, 2020 at 10:42:06PM +0200, Marek Vasut wrote:
>> On 4/28/20 10:32 AM, Lorenzo Pieralisi wrote:
>>> On Sun, Apr 26, 2020 at 02:31:47PM +0200, [e-mail] wrote:
>>>> From: Marek Vasut <[e-mail]>
>>>>
>>>> The PHY initialization function pointer does not change during the
>>>> lifetime of the driver instance, it is therefore sufficient to get
>>>> the pointer in .probe(), cache it in driver private data, and just
>>>> call the function through the cached pointer in .resume().
>>>>
>>>> Signed-off-by: Marek Vasut <[e-mail]>
>>>> Cc: Bjorn Helgaas <[e-mail]>
>>>> Cc: Lorenzo Pieralisi <[e-mail]>
>>>> Cc: Geert Uytterhoeven <[e-mail]>
>>>> Cc: Wolfram Sang <[e-mail]>
>>>> Cc: [e-mail]
>>>> ---
>>>> NOTE: Based on git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/pci.git
>>>>       branch pci/rcar
>>>> NOTE: The driver tag is now 'pcie-rcar' to distinguish it from pci-rcar-gen2.c
>>>> ---
>>>>  drivers/pci/controller/pcie-rcar.c | 10 ++++------
>>>>  1 file changed, 4 insertions(+), 6 deletions(-)
>>>
>>> Squashed in https://patchwork.kernel.org/patch/11438665
>>
>> Thanks
>>
>>> Do you want me to rename the $SUBJECT (and the branch name while at it)
>>> in the patches in my pci/rcar branch ("PCI: pcie-rcar: ...") to start
>>> the commit subject tag renaming from this cycle (and in the interim you
>>> send a rename for the drivers files ?)
>>
>> I don't really have a particular preference either way. I can keep
>> marking the drivers with pcie-rcar and pci-rcar tags if that helps
>> discern them.
> 
> So:
> 
> - "rcar" for the PCIe driver

Wouldn't it be better to mark this rcar-pcie , so it's clear it's the
PCIe driver ?

> - "rcar-pci" or "rcar-legacy" for the pci-rcar-gen2.c (preference ?
>   there is no urgency, no commit queued to rename, it is for future
>   code)

rcar-pci , since the Gen2 controller surely isn't legacy.

> Are we OK with that ? If yes I will rewrite the commits subjects
> and push out an updated pci/rcar branch.
> 
> ...DT bindings commit subjects - should I change their tag subject
> too ?

Lorenzo Pieralisi May 6, 2020, 8:57 a.m. UTC | #7

On Tue, May 05, 2020 at 08:35:04PM +0200, Marek Vasut wrote:
> On 5/5/20 8:02 PM, Lorenzo Pieralisi wrote:
> > On Fri, May 01, 2020 at 10:42:06PM +0200, Marek Vasut wrote:
> >> On 4/28/20 10:32 AM, Lorenzo Pieralisi wrote:
> >>> On Sun, Apr 26, 2020 at 02:31:47PM +0200, [e-mail] wrote:
> >>>> From: Marek Vasut <[e-mail]>
> >>>>
> >>>> The PHY initialization function pointer does not change during the
> >>>> lifetime of the driver instance, it is therefore sufficient to get
> >>>> the pointer in .probe(), cache it in driver private data, and just
> >>>> call the function through the cached pointer in .resume().
> >>>>
> >>>> Signed-off-by: Marek Vasut <[e-mail]>
> >>>> Cc: Bjorn Helgaas <[e-mail]>
> >>>> Cc: Lorenzo Pieralisi <[e-mail]>
> >>>> Cc: Geert Uytterhoeven <[e-mail]>
> >>>> Cc: Wolfram Sang <[e-mail]>
> >>>> Cc: [e-mail]
> >>>> ---
> >>>> NOTE: Based on git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/pci.git
> >>>>       branch pci/rcar
> >>>> NOTE: The driver tag is now 'pcie-rcar' to distinguish it from pci-rcar-gen2.c
> >>>> ---
> >>>>  drivers/pci/controller/pcie-rcar.c | 10 ++++------
> >>>>  1 file changed, 4 insertions(+), 6 deletions(-)
> >>>
> >>> Squashed in https://patchwork.kernel.org/patch/11438665
> >>
> >> Thanks
> >>
> >>> Do you want me to rename the $SUBJECT (and the branch name while at it)
> >>> in the patches in my pci/rcar branch ("PCI: pcie-rcar: ...") to start
> >>> the commit subject tag renaming from this cycle (and in the interim you
> >>> send a rename for the drivers files ?)
> >>
> >> I don't really have a particular preference either way. I can keep
> >> marking the drivers with pcie-rcar and pci-rcar tags if that helps
> >> discern them.
> > 
> > So:
> > 
> > - "rcar" for the PCIe driver
> 
> Wouldn't it be better to mark this rcar-pcie , so it's clear it's the
> PCIe driver ?

All other drivers in drivers/pci/controller are PCIe but don't require
an extra tag to clarify it - that's the rationale behind "rcar".

How does that sound ?

> > - "rcar-pci" or "rcar-legacy" for the pci-rcar-gen2.c (preference ?
> >   there is no urgency, no commit queued to rename, it is for future
> >   code)
> 
> rcar-pci , since the Gen2 controller surely isn't legacy.

Ok

> > Are we OK with that ? If yes I will rewrite the commits subjects
> > and push out an updated pci/rcar branch.
> > 
> > ...DT bindings commit subjects - should I change their tag subject
> > too ?

I shall leave DT bindings as they are then.

Lorenzo

Geert Uytterhoeven May 6, 2020, 9:02 a.m. UTC | #8

Hi Lorenzo,

On Wed, May 6, 2020 at 10:57 AM Lorenzo Pieralisi
<[e-mail]> wrote:
> On Tue, May 05, 2020 at 08:35:04PM +0200, Marek Vasut wrote:
> > On 5/5/20 8:02 PM, Lorenzo Pieralisi wrote:
> > > On Fri, May 01, 2020 at 10:42:06PM +0200, Marek Vasut wrote:
> > >> On 4/28/20 10:32 AM, Lorenzo Pieralisi wrote:
> > >>> On Sun, Apr 26, 2020 at 02:31:47PM +0200, [e-mail] wrote:
> > >>>> From: Marek Vasut <[e-mail]>
> > >>>>
> > >>>> The PHY initialization function pointer does not change during the
> > >>>> lifetime of the driver instance, it is therefore sufficient to get
> > >>>> the pointer in .probe(), cache it in driver private data, and just
> > >>>> call the function through the cached pointer in .resume().
> > >>>>
> > >>>> Signed-off-by: Marek Vasut <[e-mail]>
> > >>>> Cc: Bjorn Helgaas <[e-mail]>
> > >>>> Cc: Lorenzo Pieralisi <[e-mail]>
> > >>>> Cc: Geert Uytterhoeven <[e-mail]>
> > >>>> Cc: Wolfram Sang <[e-mail]>
> > >>>> Cc: [e-mail]
> > >>>> ---
> > >>>> NOTE: Based on git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/pci.git
> > >>>>       branch pci/rcar
> > >>>> NOTE: The driver tag is now 'pcie-rcar' to distinguish it from pci-rcar-gen2.c
> > >>>> ---
> > >>>>  drivers/pci/controller/pcie-rcar.c | 10 ++++------
> > >>>>  1 file changed, 4 insertions(+), 6 deletions(-)
> > >>>
> > >>> Squashed in https://patchwork.kernel.org/patch/11438665
> > >>
> > >> Thanks
> > >>
> > >>> Do you want me to rename the $SUBJECT (and the branch name while at it)
> > >>> in the patches in my pci/rcar branch ("PCI: pcie-rcar: ...") to start
> > >>> the commit subject tag renaming from this cycle (and in the interim you
> > >>> send a rename for the drivers files ?)
> > >>
> > >> I don't really have a particular preference either way. I can keep
> > >> marking the drivers with pcie-rcar and pci-rcar tags if that helps
> > >> discern them.
> > >
> > > So:
> > >
> > > - "rcar" for the PCIe driver
> >
> > Wouldn't it be better to mark this rcar-pcie , so it's clear it's the
> > PCIe driver ?
>
> All other drivers in drivers/pci/controller are PCIe but don't require
> an extra tag to clarify it - that's the rationale behind "rcar".
>
> How does that sound ?

Are there any other platforms that have two different drivers for the same
platform, one for PCI, and one for PCIe?

Gr{oetje,eeting}s,

                        Geert

Lorenzo Pieralisi May 6, 2020, 9:19 a.m. UTC | #9

On Wed, May 06, 2020 at 11:02:31AM +0200, Geert Uytterhoeven wrote:
> Hi Lorenzo,
> 
> On Wed, May 6, 2020 at 10:57 AM Lorenzo Pieralisi
> <[e-mail]> wrote:
> > On Tue, May 05, 2020 at 08:35:04PM +0200, Marek Vasut wrote:
> > > On 5/5/20 8:02 PM, Lorenzo Pieralisi wrote:
> > > > On Fri, May 01, 2020 at 10:42:06PM +0200, Marek Vasut wrote:
> > > >> On 4/28/20 10:32 AM, Lorenzo Pieralisi wrote:
> > > >>> On Sun, Apr 26, 2020 at 02:31:47PM +0200, [e-mail] wrote:
> > > >>>> From: Marek Vasut <[e-mail]>
> > > >>>>
> > > >>>> The PHY initialization function pointer does not change during the
> > > >>>> lifetime of the driver instance, it is therefore sufficient to get
> > > >>>> the pointer in .probe(), cache it in driver private data, and just
> > > >>>> call the function through the cached pointer in .resume().
> > > >>>>
> > > >>>> Signed-off-by: Marek Vasut <[e-mail]>
> > > >>>> Cc: Bjorn Helgaas <[e-mail]>
> > > >>>> Cc: Lorenzo Pieralisi <[e-mail]>
> > > >>>> Cc: Geert Uytterhoeven <[e-mail]>
> > > >>>> Cc: Wolfram Sang <[e-mail]>
> > > >>>> Cc: [e-mail]
> > > >>>> ---
> > > >>>> NOTE: Based on git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/pci.git
> > > >>>>       branch pci/rcar
> > > >>>> NOTE: The driver tag is now 'pcie-rcar' to distinguish it from pci-rcar-gen2.c
> > > >>>> ---
> > > >>>>  drivers/pci/controller/pcie-rcar.c | 10 ++++------
> > > >>>>  1 file changed, 4 insertions(+), 6 deletions(-)
> > > >>>
> > > >>> Squashed in https://patchwork.kernel.org/patch/11438665
> > > >>
> > > >> Thanks
> > > >>
> > > >>> Do you want me to rename the $SUBJECT (and the branch name while at it)
> > > >>> in the patches in my pci/rcar branch ("PCI: pcie-rcar: ...") to start
> > > >>> the commit subject tag renaming from this cycle (and in the interim you
> > > >>> send a rename for the drivers files ?)
> > > >>
> > > >> I don't really have a particular preference either way. I can keep
> > > >> marking the drivers with pcie-rcar and pci-rcar tags if that helps
> > > >> discern them.
> > > >
> > > > So:
> > > >
> > > > - "rcar" for the PCIe driver
> > >
> > > Wouldn't it be better to mark this rcar-pcie , so it's clear it's the
> > > PCIe driver ?
> >
> > All other drivers in drivers/pci/controller are PCIe but don't require
> > an extra tag to clarify it - that's the rationale behind "rcar".
> >
> > How does that sound ?
> 
> Are there any other platforms that have two different drivers for the same
> platform, one for PCI, and one for PCIe?

I don't think so - nonetheless it's time we agreed on something and be
done with it. Bjorn expressed his opinion on this and unless we have a
compelling reason not to follow it IMO it'd be better to take it.

I don't think using rcar-pcie is a disaster either.

Let me know how you want to proceed, thanks.

Lorenzo

Geert Uytterhoeven May 6, 2020, 10:22 a.m. UTC | #10

Hi Lorenzo,

On Wed, May 6, 2020 at 11:19 AM Lorenzo Pieralisi
<[e-mail]> wrote:
> On Wed, May 06, 2020 at 11:02:31AM +0200, Geert Uytterhoeven wrote:
> > On Wed, May 6, 2020 at 10:57 AM Lorenzo Pieralisi
> > <[e-mail]> wrote:
> > > On Tue, May 05, 2020 at 08:35:04PM +0200, Marek Vasut wrote:
> > > > On 5/5/20 8:02 PM, Lorenzo Pieralisi wrote:
> > > > > On Fri, May 01, 2020 at 10:42:06PM +0200, Marek Vasut wrote:
> > > > >> On 4/28/20 10:32 AM, Lorenzo Pieralisi wrote:
> > > > >>> On Sun, Apr 26, 2020 at 02:31:47PM +0200, [e-mail] wrote:
> > > > >>>> From: Marek Vasut <[e-mail]>
> > > > >>>>
> > > > >>>> The PHY initialization function pointer does not change during the
> > > > >>>> lifetime of the driver instance, it is therefore sufficient to get
> > > > >>>> the pointer in .probe(), cache it in driver private data, and just
> > > > >>>> call the function through the cached pointer in .resume().
> > > > >>>>
> > > > >>>> Signed-off-by: Marek Vasut <[e-mail]>
> > > > >>>> Cc: Bjorn Helgaas <[e-mail]>
> > > > >>>> Cc: Lorenzo Pieralisi <[e-mail]>
> > > > >>>> Cc: Geert Uytterhoeven <[e-mail]>
> > > > >>>> Cc: Wolfram Sang <[e-mail]>
> > > > >>>> Cc: [e-mail]
> > > > >>>> ---
> > > > >>>> NOTE: Based on git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/pci.git
> > > > >>>>       branch pci/rcar
> > > > >>>> NOTE: The driver tag is now 'pcie-rcar' to distinguish it from pci-rcar-gen2.c
> > > > >>>> ---
> > > > >>>>  drivers/pci/controller/pcie-rcar.c | 10 ++++------
> > > > >>>>  1 file changed, 4 insertions(+), 6 deletions(-)
> > > > >>>
> > > > >>> Squashed in https://patchwork.kernel.org/patch/11438665
> > > > >>
> > > > >> Thanks
> > > > >>
> > > > >>> Do you want me to rename the $SUBJECT (and the branch name while at it)
> > > > >>> in the patches in my pci/rcar branch ("PCI: pcie-rcar: ...") to start
> > > > >>> the commit subject tag renaming from this cycle (and in the interim you
> > > > >>> send a rename for the drivers files ?)
> > > > >>
> > > > >> I don't really have a particular preference either way. I can keep
> > > > >> marking the drivers with pcie-rcar and pci-rcar tags if that helps
> > > > >> discern them.
> > > > >
> > > > > So:
> > > > >
> > > > > - "rcar" for the PCIe driver
> > > >
> > > > Wouldn't it be better to mark this rcar-pcie , so it's clear it's the
> > > > PCIe driver ?
> > >
> > > All other drivers in drivers/pci/controller are PCIe but don't require
> > > an extra tag to clarify it - that's the rationale behind "rcar".
> > >
> > > How does that sound ?
> >
> > Are there any other platforms that have two different drivers for the same
> > platform, one for PCI, and one for PCIe?
>
> I don't think so - nonetheless it's time we agreed on something and be
> done with it. Bjorn expressed his opinion on this and unless we have a
> compelling reason not to follow it IMO it'd be better to take it.
>
> I don't think using rcar-pcie is a disaster either.
>
> Let me know how you want to proceed, thanks.

/me has just returned from a bike ride, so it's time for a bike-shed

"PCI: rcar:" for pcie-rcar.c, "PCI: rcar-gen2:" (or "PCI: rcar2"?) for
pci-rcar-gen2.c?

Gr{oetje,eeting}s,

                        Geert

Lorenzo Pieralisi May 6, 2020, 3:33 p.m. UTC | #11

On Wed, May 06, 2020 at 12:22:13PM +0200, Geert Uytterhoeven wrote:
> Hi Lorenzo,
> 
> On Wed, May 6, 2020 at 11:19 AM Lorenzo Pieralisi
> <[e-mail]> wrote:
> > On Wed, May 06, 2020 at 11:02:31AM +0200, Geert Uytterhoeven wrote:
> > > On Wed, May 6, 2020 at 10:57 AM Lorenzo Pieralisi
> > > <[e-mail]> wrote:
> > > > On Tue, May 05, 2020 at 08:35:04PM +0200, Marek Vasut wrote:
> > > > > On 5/5/20 8:02 PM, Lorenzo Pieralisi wrote:
> > > > > > On Fri, May 01, 2020 at 10:42:06PM +0200, Marek Vasut wrote:
> > > > > >> On 4/28/20 10:32 AM, Lorenzo Pieralisi wrote:
> > > > > >>> On Sun, Apr 26, 2020 at 02:31:47PM +0200, [e-mail] wrote:
> > > > > >>>> From: Marek Vasut <[e-mail]>
> > > > > >>>>
> > > > > >>>> The PHY initialization function pointer does not change during the
> > > > > >>>> lifetime of the driver instance, it is therefore sufficient to get
> > > > > >>>> the pointer in .probe(), cache it in driver private data, and just
> > > > > >>>> call the function through the cached pointer in .resume().
> > > > > >>>>
> > > > > >>>> Signed-off-by: Marek Vasut <[e-mail]>
> > > > > >>>> Cc: Bjorn Helgaas <[e-mail]>
> > > > > >>>> Cc: Lorenzo Pieralisi <[e-mail]>
> > > > > >>>> Cc: Geert Uytterhoeven <[e-mail]>
> > > > > >>>> Cc: Wolfram Sang <[e-mail]>
> > > > > >>>> Cc: [e-mail]
> > > > > >>>> ---
> > > > > >>>> NOTE: Based on git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/pci.git
> > > > > >>>>       branch pci/rcar
> > > > > >>>> NOTE: The driver tag is now 'pcie-rcar' to distinguish it from pci-rcar-gen2.c
> > > > > >>>> ---
> > > > > >>>>  drivers/pci/controller/pcie-rcar.c | 10 ++++------
> > > > > >>>>  1 file changed, 4 insertions(+), 6 deletions(-)
> > > > > >>>
> > > > > >>> Squashed in https://patchwork.kernel.org/patch/11438665
> > > > > >>
> > > > > >> Thanks
> > > > > >>
> > > > > >>> Do you want me to rename the $SUBJECT (and the branch name while at it)
> > > > > >>> in the patches in my pci/rcar branch ("PCI: pcie-rcar: ...") to start
> > > > > >>> the commit subject tag renaming from this cycle (and in the interim you
> > > > > >>> send a rename for the drivers files ?)
> > > > > >>
> > > > > >> I don't really have a particular preference either way. I can keep
> > > > > >> marking the drivers with pcie-rcar and pci-rcar tags if that helps
> > > > > >> discern them.
> > > > > >
> > > > > > So:
> > > > > >
> > > > > > - "rcar" for the PCIe driver
> > > > >
> > > > > Wouldn't it be better to mark this rcar-pcie , so it's clear it's the
> > > > > PCIe driver ?
> > > >
> > > > All other drivers in drivers/pci/controller are PCIe but don't require
> > > > an extra tag to clarify it - that's the rationale behind "rcar".
> > > >
> > > > How does that sound ?
> > >
> > > Are there any other platforms that have two different drivers for the same
> > > platform, one for PCI, and one for PCIe?
> >
> > I don't think so - nonetheless it's time we agreed on something and be
> > done with it. Bjorn expressed his opinion on this and unless we have a
> > compelling reason not to follow it IMO it'd be better to take it.
> >
> > I don't think using rcar-pcie is a disaster either.
> >
> > Let me know how you want to proceed, thanks.
> 
> /me has just returned from a bike ride, so it's time for a bike-shed
> 
> "PCI: rcar:" for pcie-rcar.c, "PCI: rcar-gen2:" (or "PCI: rcar2"?) for
> pci-rcar-gen2.c?

Fine by me, all agreed ?

Lorenzo

Bjorn Helgaas May 6, 2020, 4:59 p.m. UTC | #12

On Wed, May 06, 2020 at 04:33:40PM +0100, Lorenzo Pieralisi wrote:
> On Wed, May 06, 2020 at 12:22:13PM +0200, Geert Uytterhoeven wrote:
> > Hi Lorenzo,
> > 
> > On Wed, May 6, 2020 at 11:19 AM Lorenzo Pieralisi
> > <[e-mail]> wrote:
> > > On Wed, May 06, 2020 at 11:02:31AM +0200, Geert Uytterhoeven wrote:
> > > > On Wed, May 6, 2020 at 10:57 AM Lorenzo Pieralisi
> > > > <[e-mail]> wrote:
> > > > > On Tue, May 05, 2020 at 08:35:04PM +0200, Marek Vasut wrote:
> > > > > > On 5/5/20 8:02 PM, Lorenzo Pieralisi wrote:
> > > > > > > On Fri, May 01, 2020 at 10:42:06PM +0200, Marek Vasut wrote:
> > > > > > >> On 4/28/20 10:32 AM, Lorenzo Pieralisi wrote:
> > > > > > >>> On Sun, Apr 26, 2020 at 02:31:47PM +0200, [e-mail] wrote:
> > > > > > >>>> From: Marek Vasut <[e-mail]>
> > > > > > >>>>
> > > > > > >>>> The PHY initialization function pointer does not change during the
> > > > > > >>>> lifetime of the driver instance, it is therefore sufficient to get
> > > > > > >>>> the pointer in .probe(), cache it in driver private data, and just
> > > > > > >>>> call the function through the cached pointer in .resume().
> > > > > > >>>>
> > > > > > >>>> Signed-off-by: Marek Vasut <[e-mail]>
> > > > > > >>>> Cc: Bjorn Helgaas <[e-mail]>
> > > > > > >>>> Cc: Lorenzo Pieralisi <[e-mail]>
> > > > > > >>>> Cc: Geert Uytterhoeven <[e-mail]>
> > > > > > >>>> Cc: Wolfram Sang <[e-mail]>
> > > > > > >>>> Cc: [e-mail]
> > > > > > >>>> ---
> > > > > > >>>> NOTE: Based on git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/pci.git
> > > > > > >>>>       branch pci/rcar
> > > > > > >>>> NOTE: The driver tag is now 'pcie-rcar' to distinguish it from pci-rcar-gen2.c
> > > > > > >>>> ---
> > > > > > >>>>  drivers/pci/controller/pcie-rcar.c | 10 ++++------
> > > > > > >>>>  1 file changed, 4 insertions(+), 6 deletions(-)
> > > > > > >>>
> > > > > > >>> Squashed in https://patchwork.kernel.org/patch/11438665
> > > > > > >>
> > > > > > >> Thanks
> > > > > > >>
> > > > > > >>> Do you want me to rename the $SUBJECT (and the branch name while at it)
> > > > > > >>> in the patches in my pci/rcar branch ("PCI: pcie-rcar: ...") to start
> > > > > > >>> the commit subject tag renaming from this cycle (and in the interim you
> > > > > > >>> send a rename for the drivers files ?)
> > > > > > >>
> > > > > > >> I don't really have a particular preference either way. I can keep
> > > > > > >> marking the drivers with pcie-rcar and pci-rcar tags if that helps
> > > > > > >> discern them.
> > > > > > >
> > > > > > > So:
> > > > > > >
> > > > > > > - "rcar" for the PCIe driver
> > > > > >
> > > > > > Wouldn't it be better to mark this rcar-pcie , so it's clear it's the
> > > > > > PCIe driver ?
> > > > >
> > > > > All other drivers in drivers/pci/controller are PCIe but don't require
> > > > > an extra tag to clarify it - that's the rationale behind "rcar".
> > > > >
> > > > > How does that sound ?
> > > >
> > > > Are there any other platforms that have two different drivers for the same
> > > > platform, one for PCI, and one for PCIe?
> > >
> > > I don't think so - nonetheless it's time we agreed on something and be
> > > done with it. Bjorn expressed his opinion on this and unless we have a
> > > compelling reason not to follow it IMO it'd be better to take it.
> > >
> > > I don't think using rcar-pcie is a disaster either.
> > >
> > > Let me know how you want to proceed, thanks.
> > 
> > /me has just returned from a bike ride, so it's time for a bike-shed
> > 
> > "PCI: rcar:" for pcie-rcar.c, "PCI: rcar-gen2:" (or "PCI: rcar2"?) for
> > pci-rcar-gen2.c?
> 
> Fine by me, all agreed ?

Sounds good to me.

Patch

diff --git a/drivers/pci/controller/pcie-rcar.c b/drivers/pci/controller/pcie-rcar.c
index 1a0e74cad9bb..59e55f56e386 100644
--- a/drivers/pci/controller/pcie-rcar.c
+++ b/drivers/pci/controller/pcie-rcar.c
@@ -153,6 +153,7 @@  struct rcar_pcie {
 	int			root_bus_nr;
 	struct clk		*bus_clk;
 	struct			rcar_msi msi;
+	int			(*phy_init_fn)(struct rcar_pcie *pcie);
 };
 
 static void rcar_pci_write_reg(struct rcar_pcie *pcie, u32 val,
@@ -1147,7 +1148,6 @@  static int rcar_pcie_probe(struct platform_device *pdev)
 	struct rcar_pcie *pcie;
 	u32 data;
 	int err;
-	int (*phy_init_fn)(struct rcar_pcie *);
 	struct pci_host_bridge *bridge;
 
 	bridge = pci_alloc_host_bridge(sizeof(*pcie));
@@ -1187,8 +1187,8 @@  static int rcar_pcie_probe(struct platform_device *pdev)
 	if (err)
 		goto err_clk_disable;
 
-	phy_init_fn = of_device_get_match_data(dev);
-	err = phy_init_fn(pcie);
+	pcie->phy_init_fn = of_device_get_match_data(dev);
+	err = pcie->phy_init_fn(pcie);
 	if (err) {
 		dev_err(dev, "failed to init PCIe PHY\n");
 		goto err_clk_disable;
@@ -1253,7 +1253,6 @@  static int rcar_pcie_probe(struct platform_device *pdev)
 static int __maybe_unused rcar_pcie_resume(struct device *dev)
 {
 	struct rcar_pcie *pcie = dev_get_drvdata(dev);
-	int (*hw_init_fn)(struct rcar_pcie *);
 	unsigned int data;
 	int err;
 
@@ -1262,8 +1261,7 @@  static int __maybe_unused rcar_pcie_resume(struct device *dev)
 		return 0;
 
 	/* Failure to get a link might just be that no cards are inserted */
-	hw_init_fn = of_device_get_match_data(dev);
-	err = hw_init_fn(pcie);
+	err = pcie->phy_init_fn(pcie);
 	if (err) {
 		dev_info(dev, "PCIe link down\n");
 		return 0;