Re: [PATCH v2 2/2] PCI: mediatek-gen3: Add power and reset control feature for downstream component

[Jian Yang (杨戬)]

Dear Rob,

Sorry for the late response and thanks for your comment.

On Tue, 2023-03-21 at 11:57 -0500, Rob Herring wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> On Mon, Mar 06, 2023 at 02:40:59PM +0800, Jian Yang wrote:
> > From: "jian.yang" <jian.yang@xxxxxxxxxxxx>
> > 
> > Make MediaTek's controller driver capable of controlling power
> > supplies and reset pin of a downstream component in power-on and
> > power-off flow.
> > 
> > Some downstream components (e.g., a WIFI chip) may need an extra
> > reset other than PERST# and their power supplies, depending on
> > the requirements of platform, may need to controlled by their
> > parent's driver. To meet the requirements described above, I add
> > this
> > feature to MediaTek's PCIe controller driver as a optional feature.
> 
> If you have PCI devices with extra stuff that's not standard PCI
> stuff
> (i.e. what's on a standard connector), then you should be describing
> the PCI device in the DT.
> 
> The standard stuff should really be in the root port node rather than
> the host bridge node. That's often omitted too because many host
> bridges
> only have a single root port.

I will add a description in DT binding of this driver later.

> > 
> > Signed-off-by: jian.yang <jian.yang@xxxxxxxxxxxx>
> > ---
> >  drivers/pci/controller/pcie-mediatek-gen3.c | 86
> > ++++++++++++++++++++-
> >  1 file changed, 85 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/pci/controller/pcie-mediatek-gen3.c
> > b/drivers/pci/controller/pcie-mediatek-gen3.c
> > index b8612ce5f4d0..45e368b03ed2 100644
> > --- a/drivers/pci/controller/pcie-mediatek-gen3.c
> > +++ b/drivers/pci/controller/pcie-mediatek-gen3.c
> > @@ -8,6 +8,8 @@
> > 
> >  #include <linux/clk.h>
> >  #include <linux/delay.h>
> > +#include <linux/gpio.h>
> > +#include <linux/gpio/consumer.h>
> >  #include <linux/iopoll.h>
> >  #include <linux/irq.h>
> >  #include <linux/irqchip/chained_irq.h>
> > @@ -15,11 +17,14 @@
> >  #include <linux/kernel.h>
> >  #include <linux/module.h>
> >  #include <linux/msi.h>
> > +#include <linux/of_gpio.h>
> 
> This header is getting removed. You shouldn't depend on it.

I will remove it in V3 patch, thanks a lot.

Best regards,
Jian Yang