| Message ID | 20231106061220.21485-1-jian.yang@mediatek.com |
|---|---|
| Headers | show |
| Series | PCI: mediatek-gen3: Support controlling power supplies | expand |
On 06/11/2023 07:12, Jian Yang wrote: > From: "jian.yang" <jian.yang@mediatek.com> > > Make MediaTek's controller driver capable of controlling power > supplies and reset pin of a downstream component in power-on and > power-off process. > > 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 an optional feature. NAK, strong NAK. This should be done in a generic way because nothing here is specific to Mediatek. You just implement power sequencing of devices through quirks specific to one controller. Work with others to provide common solution. https://lpc.events/event/17/contributions/1507/ Best regards, Krzysztof
Il 06/11/23 07:12, Jian Yang ha scritto: > From: "jian.yang" <jian.yang@mediatek.com> > > Make MediaTek's controller driver capable of controlling power > supplies and reset pin of a downstream component in power-on and > power-off process. > > 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 an optional feature. > > Signed-off-by: jian.yang <jian.yang@mediatek.com> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Il 06/11/23 08:53, Krzysztof Kozlowski ha scritto: > On 06/11/2023 07:12, Jian Yang wrote: >> From: "jian.yang" <jian.yang@mediatek.com> >> >> Make MediaTek's controller driver capable of controlling power >> supplies and reset pin of a downstream component in power-on and >> power-off process. >> >> 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 an optional feature. > > NAK, strong NAK. This should be done in a generic way because nothing > here is specific to Mediatek. > > You just implement power sequencing of devices through quirks specific > to one controller. > > Work with others to provide common solution. > https://lpc.events/event/17/contributions/1507/ > I agree that working with everyone else by adding pwrseq is a must, but other other PCIe controllers are doing the exact same as this patch: if the supply and gpio names are aligned with the others, why shouldn't we let this in and then convert this driver, along with the others, to the new pwrseq subsystem when it's ready? That, because I expect the pwrseq to require a bit more time before being ready to get upstream. P.S.: Check Tegra, Broadcom, RockChip DW, IMX6Q-pcie. Cheers, Angelo
On 06/11/2023 09:36, AngeloGioacchino Del Regno wrote: > Il 06/11/23 08:53, Krzysztof Kozlowski ha scritto: >> On 06/11/2023 07:12, Jian Yang wrote: >>> From: "jian.yang" <jian.yang@mediatek.com> >>> >>> Make MediaTek's controller driver capable of controlling power >>> supplies and reset pin of a downstream component in power-on and >>> power-off process. >>> >>> 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 an optional feature. >> >> NAK, strong NAK. This should be done in a generic way because nothing >> here is specific to Mediatek. >> >> You just implement power sequencing of devices through quirks specific >> to one controller. >> >> Work with others to provide common solution. >> https://lpc.events/event/17/contributions/1507/ >> > > I agree that working with everyone else by adding pwrseq is a must, but other > other PCIe controllers are doing the exact same as this patch: if the supply > and gpio names are aligned with the others, why shouldn't we let this in and > then convert this driver, along with the others, to the new pwrseq subsystem > when it's ready? Because you already push to the PCI controller bindings new properties which are not properties of the PCI controller. > > That, because I expect the pwrseq to require a bit more time before being > ready to get upstream. > > P.S.: Check Tegra, Broadcom, RockChip DW, IMX6Q-pcie. Every new hack will not make it faster. :( At some point one have to say - enough of hacks, start doing it properly with upstream. Best regards, Krzysztof
Il 06/11/23 09:46, Krzysztof Kozlowski ha scritto: > On 06/11/2023 09:36, AngeloGioacchino Del Regno wrote: >> Il 06/11/23 08:53, Krzysztof Kozlowski ha scritto: >>> On 06/11/2023 07:12, Jian Yang wrote: >>>> From: "jian.yang" <jian.yang@mediatek.com> >>>> >>>> Make MediaTek's controller driver capable of controlling power >>>> supplies and reset pin of a downstream component in power-on and >>>> power-off process. >>>> >>>> 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 an optional feature. >>> >>> NAK, strong NAK. This should be done in a generic way because nothing >>> here is specific to Mediatek. >>> >>> You just implement power sequencing of devices through quirks specific >>> to one controller. >>> >>> Work with others to provide common solution. >>> https://lpc.events/event/17/contributions/1507/ >>> >> >> I agree that working with everyone else by adding pwrseq is a must, but other >> other PCIe controllers are doing the exact same as this patch: if the supply >> and gpio names are aligned with the others, why shouldn't we let this in and >> then convert this driver, along with the others, to the new pwrseq subsystem >> when it's ready? > > Because you already push to the PCI controller bindings new properties > which are not properties of the PCI controller. > >> >> That, because I expect the pwrseq to require a bit more time before being >> ready to get upstream. >> >> P.S.: Check Tegra, Broadcom, RockChip DW, IMX6Q-pcie. > > Every new hack will not make it faster. :( At some point one have to say > - enough of hacks, start doing it properly with upstream. > Eh, that's a fair point. I can't really disagree with that. Cheers, Angelo
Add support for controlling power supplies and reset GPIO of a downstream component in Mediatek's PCIe GEN3 controller driver. Changes in v4: 1. Rename power supplies properties in DT binding and driver. 2. Reorder variables alphabetically. 3. Use 'dev_err_probe' to do some error handling stuff. Changes in v3: 1. Modify description of power supply properties in DT binding. 2. Remove unused header files. 3. Use 'device_wakeup_path' to determine whether the downstream component needs to skip the reset process in system suspend scenarios. Changes in v2: 1. Remove an unnecessary property in dt-bindings file. 2. Use the flag 'GPIOD_OUT_LOW' to set initial state of a downstream component's reset GPIO. 3. Keep downstream component powered on in suspend state if it is capable of waking up the system. jian.yang (2): dt-bindings: PCI: mediatek-gen3: Add support for controlling power and reset PCI: mediatek-gen3: Add power and reset control feature for downstream component .../bindings/pci/mediatek-pcie-gen3.yaml | 30 +++++++ drivers/pci/controller/pcie-mediatek-gen3.c | 89 ++++++++++++++++++- 2 files changed, 118 insertions(+), 1 deletion(-)