On Mon, Nov 20, 2017 at 03:30:52PM -0600, Bjorn Helgaas wrote:
[...]
> > static inline struct tegra_msi *to_tegra_msi(struct msi_controller *chip)
> > @@ -2290,6 +2293,16 @@ static void tegra_pcie_apply_sw_fixup(struct tegra_pcie_port *port)
> > (7 << RP_L1_PM_SUBSTATES_CTL_T_PWRN_VAL_SHIFT);
> > writel(value, port->base + RP_L1_PM_SUBSTATES_CTL);
> >
> > + if (soc->l1ss_rp_wake_fixup) {
> > + /* Set CLKREQ asserted delay greater than Power_Off
> > + * time (2us) to avoid RP wakeup in L1.2_ENTRY
> > + */
>
> Use standard multi-line comment formatting (and "L1.2.Entry" as above).
>
> I assume "CLKREQ asserted delay" is a Tegra-internal thing, since it
> doesn't obviously correspond to a timing parameter in the spec.
>
> And I assume it doesn't depend on any of the OS-writable things in the
> L1 PM Substates control registers, since this fixup isn't executed
> during config writes to those registers?
>
> Does this depend on any circuit details outside of Tegra, i.e., should
> it be described via DT?
Yes, I have noticed too that there are lots of hardcoded values in this
driver that may benefit from some Tegra DT bindings instead of
hardcoding everything in the kernel.
Thanks,
Lorenzo
> > + value = readl(port->base + RP_L1_PM_SUBSTATES_1_CTL);
> > + value &= ~RP_L1SS_1_CTL_CLKREQ_ASSERTED_DLY_MASK;
> > + value |= RP_L1SS_1_CTL_CLKREQ_ASSERTED_DLY;
> > + writel(value, port->base + RP_L1_PM_SUBSTATES_1_CTL);
> > + }
> > +
> > /* Following is based on clk_m being 19.2 MHz */
> > value = readl(port->base + RP_L1_PM_SUBSTATES_1_CTL);
> > value &= ~RP_L1_PM_SUBSTATES_1_CTL_PWR_OFF_DLY_MASK;
> > @@ -2446,6 +2459,7 @@ static const struct tegra_pcie_soc tegra20_pcie = {
> > .program_deskew_time = false,
> > .updateFC_threshold = false,
> > .has_aspm_l1ss = false,
> > + .l1ss_rp_wake_fixup = false,
> > };
> >
> > static const struct tegra_pcie_soc tegra30_pcie = {
> > @@ -2468,6 +2482,7 @@ static const struct tegra_pcie_soc tegra30_pcie = {
> > .program_deskew_time = false,
> > .updateFC_threshold = false,
> > .has_aspm_l1ss = false,
> > + .l1ss_rp_wake_fixup = false,
> > };
> >
> > static const struct tegra_pcie_soc tegra124_pcie = {
> > @@ -2489,6 +2504,7 @@ static const struct tegra_pcie_soc tegra124_pcie = {
> > .program_deskew_time = false,
> > .updateFC_threshold = false,
> > .has_aspm_l1ss = false,
> > + .l1ss_rp_wake_fixup = false,
> > };
> >
> > static const struct tegra_pcie_soc tegra210_pcie = {
> > @@ -2518,6 +2534,7 @@ static const struct tegra_pcie_soc tegra210_pcie = {
> > .program_deskew_time = true,
> > .updateFC_threshold = true,
> > .has_aspm_l1ss = true,
> > + .l1ss_rp_wake_fixup = true,
> > };
> >
> > static const struct tegra_pcie_soc tegra186_pcie = {
> > @@ -2540,6 +2557,7 @@ static const struct tegra_pcie_soc tegra186_pcie = {
> > .program_deskew_time = false,
> > .updateFC_threshold = false,
> > .has_aspm_l1ss = true,
> > + .l1ss_rp_wake_fixup = false,
> > };
> >
> > static const struct of_device_id tegra_pcie_of_match[] = {
> > --
> > 2.7.4
> >
