[U-Boot] [PATCH 1/5] pci: tegra: port to standard clock/reset/pwr domain APIs
Simon Glass
sjg at chromium.org
Fri Aug 5 03:36:57 CEST 2016
Hi Stephen,
On 4 August 2016 at 12:52, Stephen Warren <swarren at wwwdotorg.org> wrote:
> On 08/03/2016 07:16 PM, Simon Glass wrote:
>>
>> Hi Stephen,
>>
>> On 1 August 2016 at 09:22, Stephen Warren <swarren at wwwdotorg.org> wrote:
>>>
>>> On 07/31/2016 07:02 PM, Simon Glass wrote:
>>>>
>>>>
>>>> Hi Stephen,
>>>>
>>>> On 27 July 2016 at 15:48, Stephen Warren <swarren at wwwdotorg.org> wrote:
>>>>>
>>>>>
>>>>> From: Stephen Warren <swarren at nvidia.com>
>>>>>
>>>>> Tegra186 supports the new standard clock, reset, and power domain APIs.
>>>>> Older Tegra SoCs still use custom APIs. Enhance the Tegra PCIe driver
>>>>> so
>>>>> that it can operate with either set of APIs.
>>>>>
>>>>> On Tegra186, the BPMP handles all aspects of PCIe PHY (UPHY)
>>>>> programming.
>>>>> Consequently, this logic is disabled too.
>>>>>
>>>>> Signed-off-by: Stephen Warren <swarren at nvidia.com>
>>>>> ---
>>>>> This whole series builds on the other Tegra186 series that I just sent.
>>>>>
>>>>> drivers/pci/Kconfig | 1 +
>>>>> drivers/pci/pci_tegra.c | 154
>>>>> ++++++++++++++++++++++++++++++++++++++++++++++--
>>>>> 2 files changed, 150 insertions(+), 5 deletions(-)
>>>>>
>>>>> diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
>>>>> index 26aa2b0930a0..669e37bb5dc5 100644
>>>>> --- a/drivers/pci/Kconfig
>>>>> +++ b/drivers/pci/Kconfig
>>>>> @@ -31,6 +31,7 @@ config PCI_SANDBOX
>>>>> config PCI_TEGRA
>>>>> bool "Tegra PCI support"
>>>>> depends on TEGRA
>>>>> + depends on (TEGRA186 && POWER_DOMAIN) || (!TEGRA186)
>>>>> help
>>>>> Enable support for the PCIe controller found on some
>>>>> generations of
>>>>> Tegra. Tegra20 has 2 root ports with a total of 4 lanes,
>>>>> Tegra30 has
>>>>> diff --git a/drivers/pci/pci_tegra.c b/drivers/pci/pci_tegra.c
>>>>> index 352cdef56ab4..a6785ad0bbee 100644
>>>>> --- a/drivers/pci/pci_tegra.c
>>>>> +++ b/drivers/pci/pci_tegra.c
>>>>> @@ -13,22 +13,26 @@
>>>>> #define pr_fmt(fmt) "tegra-pcie: " fmt
>>>>>
>>>>> #include <common.h>
>>>>> +#include <clk.h>
>>>>> #include <dm.h>
>>>>> #include <errno.h>
>>>>> #include <fdtdec.h>
>>>>> #include <malloc.h>
>>>>> #include <pci.h>
>>>>> +#include <power-domain.h>
>>>>> +#include <reset.h>
>>>>>
>>>>> #include <asm/io.h>
>>>>> #include <asm/gpio.h>
>>>>>
>>>>> +#include <linux/list.h>
>>>>> +
>>>>> +#ifndef CONFIG_TEGRA186
>>>>> #include <asm/arch/clock.h>
>>>>> #include <asm/arch/powergate.h>
>>>>> #include <asm/arch-tegra/xusb-padctl.h>
>>>>> -
>>>>> -#include <linux/list.h>
>>>>> -
>>>>> #include <dt-bindings/pinctrl/pinctrl-tegra-xusb.h>
>>>>> +#endif
>>>>>
>>>>> DECLARE_GLOBAL_DATA_PTR;
>>>>>
>>>>> @@ -103,6 +107,9 @@ DECLARE_GLOBAL_DATA_PTR;
>>>>> #define AFI_PCIE_CONFIG_SM2TMS0_XBAR_CONFIG_222 (0x1 << 20)
>>>>> #define AFI_PCIE_CONFIG_SM2TMS0_XBAR_CONFIG_X4_X1 (0x1 << 20)
>>>>> #define AFI_PCIE_CONFIG_SM2TMS0_XBAR_CONFIG_411 (0x2 << 20)
>>>>> +#define AFI_PCIE_CONFIG_SM2TMS0_XBAR_CONFIG_T186_401 (0x0 << 20)
>>>>> +#define AFI_PCIE_CONFIG_SM2TMS0_XBAR_CONFIG_T186_211 (0x1 << 20)
>>>>> +#define AFI_PCIE_CONFIG_SM2TMS0_XBAR_CONFIG_T186_111 (0x2 << 20)
>>>>>
>>>>> #define AFI_FUSE 0x104
>>>>> #define AFI_FUSE_PCIE_T0_GEN2_DIS (1 << 2)
>>>>> @@ -110,6 +117,7 @@ DECLARE_GLOBAL_DATA_PTR;
>>>>> #define AFI_PEX0_CTRL 0x110
>>>>> #define AFI_PEX1_CTRL 0x118
>>>>> #define AFI_PEX2_CTRL 0x128
>>>>> +#define AFI_PEX2_CTRL_T186 0x19c
>>>>> #define AFI_PEX_CTRL_RST (1 << 0)
>>>>> #define AFI_PEX_CTRL_CLKREQ_EN (1 << 1)
>>>>> #define AFI_PEX_CTRL_REFCLK_EN (1 << 3)
>>>>> @@ -173,6 +181,7 @@ enum tegra_pci_id {
>>>>> TEGRA30_PCIE,
>>>>> TEGRA124_PCIE,
>>>>> TEGRA210_PCIE,
>>>>> + TEGRA186_PCIE,
>>>>> };
>>>>>
>>>>> struct tegra_pcie_port {
>>>>> @@ -189,6 +198,7 @@ struct tegra_pcie_soc {
>>>>> unsigned int num_ports;
>>>>> unsigned long pads_pll_ctl;
>>>>> unsigned long tx_ref_sel;
>>>>> + unsigned long afi_pex2_ctrl;
>>>>> u32 pads_refclk_cfg0;
>>>>> u32 pads_refclk_cfg1;
>>>>> bool has_pex_clkreq_en;
>>>>> @@ -209,7 +219,17 @@ struct tegra_pcie {
>>>>> unsigned long xbar;
>>>>>
>>>>> const struct tegra_pcie_soc *soc;
>>>>> +
>>>>> +#ifdef CONFIG_TEGRA186
>>>>> + struct clk clk_afi;
>>>>> + struct clk clk_pex;
>>>>> + struct reset_ctl reset_afi;
>>>>> + struct reset_ctl reset_pex;
>>>>> + struct reset_ctl reset_pcie_x;
>>>>> + struct power_domain pwrdom;
>>>>> +#else
>>>>
>>>>
>>>>
>>>> Eek. This is a compile-time driver configuration. Can you use the
>>>> device tree compatible string to detect which to use?
>>>
>>>
>>>
>>> No. The legacy APIs are simply not available (not compiled in) on the
>>> newer
>>> SoCs, so there must be a compile-time condition.
>>>
>>> It is theoretically possible to convert all the old SoCs to the new APIs
>>> so
>>> that this conditional goes away in the future, but we don't have that
>>> work
>>> scheduled at present.
>>
>>
>> What did you do with Linux?
>
>
> We lucked out and life was a bit simpler:
>
> When we started adding Tegra support, there was already a standard clock API
> header, albeit with an entirely separate implementation for each SoC. When
> the common clock framework/core appeared, it maintained this same API so
> drivers weren't affected, except for a change in a #include filename.
>
> As far as converting the clock driver from a standalone/custom
> implementation to being based on the common clock framework/core, life was
> much easier. At that time, we only supported 2 Tegra SoC generations and
> many fewer boards and drivers, and so significantly less testing was
> required for the converted implementation. Many SoC generations, board, and
> drivers were only written once the new clock implementation was in place, so
> never needed conversion.
>
> Tegra started out with a completely custom reset API and implementation.
> When Tegra converted from its custom standalone clock implementation to the
> common clock framework/core, that same API was re-implemented using the new
> clock code (as you know, clocks and resets on Tegra are very closely tied),
> and so there was no effect on drivers.
>
> Later, we converted to the new reset API/framework/core. At this point, we
> did support 4 SoC generations and many more boards. However, IIRC during
> this time, there was a huge focus on converting to standard APIs, and a lull
> in new SoC and board support, so there was time to spend on large-scale
> conversions; they didn't take time away from other work required to support
> new SoCs/boards. This conversion implemented the new standard reset for all
> SoCs first, converted all drivers to use the new API roughly at once, and
> then removed the old API.
>
> So we got away without ifdefs.
>
> In U-Boot, we now support many more SoCs, boards, and perhaps even drivers,
> so a "convert everything at once" approach is quite a lot more work than it
> was in the kernel, especially on the clock/reset driver implementation side.
> Equally, to be honest, my primary focus is currently on adding Tegra186
> support at the moment. Given we needed a completely new clock/reset/...
> implementation anyway due to the existence of the BPMP, it was a no-brainer
> to create and/or use the new standard clock/reset/... APIs for Tegra186,
> rather than basing the new implementation on the old custom APIs. However,
> converting all the other SoCs/boards first is just going to delay any
> forward progress on Tegra186 and lead to a mega series of code conversions
> that I'd rather avoid doing all at once. Adding these ifdefs, allows
> incremental forward progress to be made pretty easily. They should be
> removed when older SoC generations are converted to the new APIs, so
> everything is unified again. Granted, I have no definite timescale for that
> at present.
OK, well I'd like to see some TODOs in the code around these #ifdefs.
They are definitely not what we want, and should be marked as interim.
Please see if you can find someone to convert things over. I can
perhaps help too.
Regards,
Simon
More information about the U-Boot
mailing list