2024-01-23 14:48:01

by Bibek Kumar Patro

[permalink] [raw]
Subject: [PATCH v9 4/5] iommu/arm-smmu: add ACTLR data and support for SM8550

Add ACTLR data table for SM8550 along with support for
same including SM8550 specific implementation operations.

Signed-off-by: Bibek Kumar Patro <[email protected]>
---
drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 90 ++++++++++++++++++++++
1 file changed, 90 insertions(+)

diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
index 6004c6d9a7b2..db15b1eade97 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
@@ -23,6 +23,86 @@

#define CPRE (1 << 1)
#define CMTLB (1 << 0)
+#define PREFETCH_SHIFT 8
+#define PREFETCH_DEFAULT 0
+#define PREFETCH_SHALLOW (1 << PREFETCH_SHIFT)
+#define PREFETCH_MODERATE (2 << PREFETCH_SHIFT)
+#define PREFETCH_DEEP (3 << PREFETCH_SHIFT)
+#define PREFETCH_SWITCH_GFX (5 << 3)
+
+static const struct actlr_config sm8550_apps_actlr_cfg[] = {
+ { 0x18a0, 0x0000, PREFETCH_SHALLOW | CPRE | CMTLB },
+ { 0x18e0, 0x0000, PREFETCH_SHALLOW | CPRE | CMTLB },
+ { 0x0800, 0x0020, PREFETCH_DEFAULT | CMTLB },
+ { 0x1800, 0x00c0, PREFETCH_DEFAULT | CMTLB },
+ { 0x1820, 0x0000, PREFETCH_DEFAULT | CMTLB },
+ { 0x1860, 0x0000, PREFETCH_DEFAULT | CMTLB },
+ { 0x0c01, 0x0020, PREFETCH_DEEP | CPRE | CMTLB },
+ { 0x0c02, 0x0020, PREFETCH_DEEP | CPRE | CMTLB },
+ { 0x0c03, 0x0020, PREFETCH_DEEP | CPRE | CMTLB },
+ { 0x0c04, 0x0020, PREFETCH_DEEP | CPRE | CMTLB },
+ { 0x0c05, 0x0020, PREFETCH_DEEP | CPRE | CMTLB },
+ { 0x0c06, 0x0020, PREFETCH_DEEP | CPRE | CMTLB },
+ { 0x0c07, 0x0020, PREFETCH_DEEP | CPRE | CMTLB },
+ { 0x0c08, 0x0020, PREFETCH_DEEP | CPRE | CMTLB },
+ { 0x0c09, 0x0020, PREFETCH_DEEP | CPRE | CMTLB },
+ { 0x0c0c, 0x0020, PREFETCH_DEEP | CPRE | CMTLB },
+ { 0x0c0d, 0x0020, PREFETCH_DEEP | CPRE | CMTLB },
+ { 0x0c0e, 0x0020, PREFETCH_DEEP | CPRE | CMTLB },
+ { 0x0c0f, 0x0020, PREFETCH_DEEP | CPRE | CMTLB },
+ { 0x1961, 0x0000, PREFETCH_DEEP | CPRE | CMTLB },
+ { 0x1962, 0x0000, PREFETCH_DEEP | CPRE | CMTLB },
+ { 0x1963, 0x0000, PREFETCH_DEEP | CPRE | CMTLB },
+ { 0x1964, 0x0000, PREFETCH_DEEP | CPRE | CMTLB },
+ { 0x1965, 0x0000, PREFETCH_DEEP | CPRE | CMTLB },
+ { 0x1966, 0x0000, PREFETCH_DEEP | CPRE | CMTLB },
+ { 0x1967, 0x0000, PREFETCH_DEEP | CPRE | CMTLB },
+ { 0x1968, 0x0000, PREFETCH_DEEP | CPRE | CMTLB },
+ { 0x1969, 0x0000, PREFETCH_DEEP | CPRE | CMTLB },
+ { 0x196c, 0x0000, PREFETCH_DEEP | CPRE | CMTLB },
+ { 0x196d, 0x0000, PREFETCH_DEEP | CPRE | CMTLB },
+ { 0x196e, 0x0000, PREFETCH_DEEP | CPRE | CMTLB },
+ { 0x196f, 0x0000, PREFETCH_DEEP | CPRE | CMTLB },
+ { 0x19c1, 0x0010, PREFETCH_DEEP | CPRE | CMTLB },
+ { 0x19c2, 0x0010, PREFETCH_DEEP | CPRE | CMTLB },
+ { 0x19c3, 0x0010, PREFETCH_DEEP | CPRE | CMTLB },
+ { 0x19c4, 0x0010, PREFETCH_DEEP | CPRE | CMTLB },
+ { 0x19c5, 0x0010, PREFETCH_DEEP | CPRE | CMTLB },
+ { 0x19c6, 0x0010, PREFETCH_DEEP | CPRE | CMTLB },
+ { 0x19c7, 0x0010, PREFETCH_DEEP | CPRE | CMTLB },
+ { 0x19c8, 0x0010, PREFETCH_DEEP | CPRE | CMTLB },
+ { 0x19c9, 0x0010, PREFETCH_DEEP | CPRE | CMTLB },
+ { 0x19cc, 0x0010, PREFETCH_DEEP | CPRE | CMTLB },
+ { 0x19cd, 0x0010, PREFETCH_DEEP | CPRE | CMTLB },
+ { 0x19ce, 0x0010, PREFETCH_DEEP | CPRE | CMTLB },
+ { 0x19cf, 0x0010, PREFETCH_DEEP | CPRE | CMTLB },
+ { 0x1c00, 0x0002, PREFETCH_SHALLOW | CPRE | CMTLB },
+ { 0x1c01, 0x0000, PREFETCH_DEFAULT | CMTLB },
+ { 0x1920, 0x0000, PREFETCH_SHALLOW | CPRE | CMTLB },
+ { 0x1923, 0x0000, PREFETCH_SHALLOW | CPRE | CMTLB },
+ { 0x1924, 0x0000, PREFETCH_SHALLOW | CPRE | CMTLB },
+ { 0x1940, 0x0000, PREFETCH_SHALLOW | CPRE | CMTLB },
+ { 0x1941, 0x0004, PREFETCH_SHALLOW | CPRE | CMTLB },
+ { 0x1943, 0x0000, PREFETCH_SHALLOW | CPRE | CMTLB },
+ { 0x1944, 0x0000, PREFETCH_SHALLOW | CPRE | CMTLB },
+ { 0x1947, 0x0000, PREFETCH_SHALLOW | CPRE | CMTLB },
+};
+
+static const struct actlr_config sm8550_gfx_actlr_cfg[] = {
+ { 0x0000, 0x03ff, PREFETCH_SWITCH_GFX | PREFETCH_DEEP | CPRE | CMTLB },
+};
+
+static const struct actlr_variant sm8550_actlr[] = {
+ {
+ .io_start = 0x15000000,
+ .actlrcfg = sm8550_apps_actlr_cfg,
+ .num_actlrcfg = ARRAY_SIZE(sm8550_apps_actlr_cfg)
+ }, {
+ .io_start = 0x03da0000,
+ .actlrcfg = sm8550_gfx_actlr_cfg,
+ .num_actlrcfg = ARRAY_SIZE(sm8550_gfx_actlr_cfg)
+ },
+};

static struct qcom_smmu *to_qcom_smmu(struct arm_smmu_device *smmu)
{
@@ -595,6 +675,15 @@ static const struct qcom_smmu_match_data sdm845_smmu_500_data = {
/* Also no debug configuration. */
};

+
+static const struct qcom_smmu_match_data sm8550_smmu_500_impl0_data = {
+ .impl = &qcom_smmu_500_impl,
+ .adreno_impl = &qcom_adreno_smmu_500_impl,
+ .cfg = &qcom_smmu_impl0_cfg,
+ .actlrvar = sm8550_actlr,
+ .num_smmu = ARRAY_SIZE(sm8550_actlr),
+};
+
static const struct qcom_smmu_match_data qcom_smmu_500_impl0_data = {
.impl = &qcom_smmu_500_impl,
.adreno_impl = &qcom_adreno_smmu_500_impl,
@@ -629,6 +718,7 @@ static const struct of_device_id __maybe_unused qcom_smmu_impl_of_match[] = {
{ .compatible = "qcom,sm8250-smmu-500", .data = &qcom_smmu_500_impl0_data },
{ .compatible = "qcom,sm8350-smmu-500", .data = &qcom_smmu_500_impl0_data },
{ .compatible = "qcom,sm8450-smmu-500", .data = &qcom_smmu_500_impl0_data },
+ { .compatible = "qcom,sm8550-smmu-500", .data = &sm8550_smmu_500_impl0_data },
{ .compatible = "qcom,smmu-500", .data = &qcom_smmu_500_impl0_data },
{ }
};
--
2.17.1



2024-01-23 18:42:39

by Konrad Dybcio

[permalink] [raw]
Subject: Re: [PATCH v9 4/5] iommu/arm-smmu: add ACTLR data and support for SM8550



On 1/23/24 15:45, Bibek Kumar Patro wrote:
> Add ACTLR data table for SM8550 along with support for
> same including SM8550 specific implementation operations.
>
> Signed-off-by: Bibek Kumar Patro <[email protected]>
> ---

Reviewed-by: Konrad Dybcio <[email protected]>

Konrad

2024-02-13 13:54:36

by Will Deacon

[permalink] [raw]
Subject: Re: [PATCH v9 4/5] iommu/arm-smmu: add ACTLR data and support for SM8550

On Tue, Jan 23, 2024 at 08:15:42PM +0530, Bibek Kumar Patro wrote:
> Add ACTLR data table for SM8550 along with support for
> same including SM8550 specific implementation operations.
>
> Signed-off-by: Bibek Kumar Patro <[email protected]>
> ---
> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 90 ++++++++++++++++++++++
> 1 file changed, 90 insertions(+)
>
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> index 6004c6d9a7b2..db15b1eade97 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> @@ -23,6 +23,86 @@
>
> #define CPRE (1 << 1)
> #define CMTLB (1 << 0)
> +#define PREFETCH_SHIFT 8
> +#define PREFETCH_DEFAULT 0
> +#define PREFETCH_SHALLOW (1 << PREFETCH_SHIFT)
> +#define PREFETCH_MODERATE (2 << PREFETCH_SHIFT)
> +#define PREFETCH_DEEP (3 << PREFETCH_SHIFT)
> +#define PREFETCH_SWITCH_GFX (5 << 3)
> +
> +static const struct actlr_config sm8550_apps_actlr_cfg[] = {
> + { 0x18a0, 0x0000, PREFETCH_SHALLOW | CPRE | CMTLB },
> + { 0x18e0, 0x0000, PREFETCH_SHALLOW | CPRE | CMTLB },
> + { 0x0800, 0x0020, PREFETCH_DEFAULT | CMTLB },
> + { 0x1800, 0x00c0, PREFETCH_DEFAULT | CMTLB },
> + { 0x1820, 0x0000, PREFETCH_DEFAULT | CMTLB },
> + { 0x1860, 0x0000, PREFETCH_DEFAULT | CMTLB },
> + { 0x0c01, 0x0020, PREFETCH_DEEP | CPRE | CMTLB },
> + { 0x0c02, 0x0020, PREFETCH_DEEP | CPRE | CMTLB },
> + { 0x0c03, 0x0020, PREFETCH_DEEP | CPRE | CMTLB },
> + { 0x0c04, 0x0020, PREFETCH_DEEP | CPRE | CMTLB },
> + { 0x0c05, 0x0020, PREFETCH_DEEP | CPRE | CMTLB },
> + { 0x0c06, 0x0020, PREFETCH_DEEP | CPRE | CMTLB },

[...]

Isn't this effectively hard-coding the topology of the SoC in the driver?
Wouldn't it better describing higher-level prefetch properties in the DT
nodes corresponding to the upstream devices?

Looking back at the prior revisions of this series, it seems like others
were in favour of this approach, so if that's the general consensus, then
so be it. But is this _really_ what we want in the SMMU driver? It would
be good to have an Ack from Robin and a DT maintainer on this mechanism.

It just all feels a bit like a step back into the bad old world of
platform data to me, where we end up trying to maintain a bunch of random
constants that supposedly make things faster for somebody :/

Will

2024-02-21 08:56:33

by Bibek Kumar Patro

[permalink] [raw]
Subject: Re: [PATCH v9 4/5] iommu/arm-smmu: add ACTLR data and support for SM8550



On 2/13/2024 7:17 PM, Will Deacon wrote:
> On Tue, Jan 23, 2024 at 08:15:42PM +0530, Bibek Kumar Patro wrote:
>> Add ACTLR data table for SM8550 along with support for
>> same including SM8550 specific implementation operations.
>>
>> Signed-off-by: Bibek Kumar Patro <[email protected]>
>> ---
>> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 90 ++++++++++++++++++++++
>> 1 file changed, 90 insertions(+)
>>
>> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>> index 6004c6d9a7b2..db15b1eade97 100644
>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>> @@ -23,6 +23,86 @@
>>
>> #define CPRE (1 << 1)
>> #define CMTLB (1 << 0)
>> +#define PREFETCH_SHIFT 8
>> +#define PREFETCH_DEFAULT 0
>> +#define PREFETCH_SHALLOW (1 << PREFETCH_SHIFT)
>> +#define PREFETCH_MODERATE (2 << PREFETCH_SHIFT)
>> +#define PREFETCH_DEEP (3 << PREFETCH_SHIFT)
>> +#define PREFETCH_SWITCH_GFX (5 << 3)
>> +
>> +static const struct actlr_config sm8550_apps_actlr_cfg[] = {
>> + { 0x18a0, 0x0000, PREFETCH_SHALLOW | CPRE | CMTLB },
>> + { 0x18e0, 0x0000, PREFETCH_SHALLOW | CPRE | CMTLB },
>> + { 0x0800, 0x0020, PREFETCH_DEFAULT | CMTLB },
>> + { 0x1800, 0x00c0, PREFETCH_DEFAULT | CMTLB },
>> + { 0x1820, 0x0000, PREFETCH_DEFAULT | CMTLB },
>> + { 0x1860, 0x0000, PREFETCH_DEFAULT | CMTLB },
>> + { 0x0c01, 0x0020, PREFETCH_DEEP | CPRE | CMTLB },
>> + { 0x0c02, 0x0020, PREFETCH_DEEP | CPRE | CMTLB },
>> + { 0x0c03, 0x0020, PREFETCH_DEEP | CPRE | CMTLB },
>> + { 0x0c04, 0x0020, PREFETCH_DEEP | CPRE | CMTLB },
>> + { 0x0c05, 0x0020, PREFETCH_DEEP | CPRE | CMTLB },
>> + { 0x0c06, 0x0020, PREFETCH_DEEP | CPRE | CMTLB },
>
> [...]
>
> Isn't this effectively hard-coding the topology of the SoC in the driver?
> Wouldn't it better describing higher-level prefetch properties in the DT
> nodes corresponding to the upstream devices?

Since prefetch data stored in this table represent settings for the
ACTLR register, and doesn't exactly define the hardware (So in this
manner prefetch data won't exactly be a part of soc topology ?).
So it seemed apt not to use the device tree for storing the prefetch
property. Hence we reverted from the DT approach (initial proposal in
RFC to piggyback on iommus property to store prefetch settings) back to
use driver for storing this data.

Some drivers use the same approach for storing their platform specific
data. Examples being
drivers/phy/qualcomm/phy-qcom-qmp-combo.c
drivers/soc/qcom/llcc-qcom.c
These drivers were taken as reference for storing platform specific
ACTLR data.

Thanks & regards,
Bibek

>
> Looking back at the prior revisions of this series, it seems like others
> were in favour of this approach, so if that's the general consensus, then
> so be it. But is this _really_ what we want in the SMMU driver? It would
> be good to have an Ack from Robin and a DT maintainer on this mechanism.
>
> It just all feels a bit like a step back into the bad old world of
> platform data to me, where we end up trying to maintain a bunch of random
> constants that supposedly make things faster for somebody :/
> > Will

2024-02-21 13:21:48

by Will Deacon

[permalink] [raw]
Subject: Re: [PATCH v9 4/5] iommu/arm-smmu: add ACTLR data and support for SM8550

On Wed, Feb 21, 2024 at 02:25:26PM +0530, Bibek Kumar Patro wrote:
> On 2/13/2024 7:17 PM, Will Deacon wrote:
> > On Tue, Jan 23, 2024 at 08:15:42PM +0530, Bibek Kumar Patro wrote:
> > > Add ACTLR data table for SM8550 along with support for
> > > same including SM8550 specific implementation operations.
> > >
> > > Signed-off-by: Bibek Kumar Patro <[email protected]>
> > > ---
> > > drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 90 ++++++++++++++++++++++
> > > 1 file changed, 90 insertions(+)
> > >
> > > diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> > > index 6004c6d9a7b2..db15b1eade97 100644
> > > --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> > > +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> > > @@ -23,6 +23,86 @@
> > >
> > > #define CPRE (1 << 1)
> > > #define CMTLB (1 << 0)
> > > +#define PREFETCH_SHIFT 8
> > > +#define PREFETCH_DEFAULT 0
> > > +#define PREFETCH_SHALLOW (1 << PREFETCH_SHIFT)
> > > +#define PREFETCH_MODERATE (2 << PREFETCH_SHIFT)
> > > +#define PREFETCH_DEEP (3 << PREFETCH_SHIFT)
> > > +#define PREFETCH_SWITCH_GFX (5 << 3)
> > > +
> > > +static const struct actlr_config sm8550_apps_actlr_cfg[] = {
> > > + { 0x18a0, 0x0000, PREFETCH_SHALLOW | CPRE | CMTLB },
> > > + { 0x18e0, 0x0000, PREFETCH_SHALLOW | CPRE | CMTLB },
> > > + { 0x0800, 0x0020, PREFETCH_DEFAULT | CMTLB },
> > > + { 0x1800, 0x00c0, PREFETCH_DEFAULT | CMTLB },
> > > + { 0x1820, 0x0000, PREFETCH_DEFAULT | CMTLB },
> > > + { 0x1860, 0x0000, PREFETCH_DEFAULT | CMTLB },
> > > + { 0x0c01, 0x0020, PREFETCH_DEEP | CPRE | CMTLB },
> > > + { 0x0c02, 0x0020, PREFETCH_DEEP | CPRE | CMTLB },
> > > + { 0x0c03, 0x0020, PREFETCH_DEEP | CPRE | CMTLB },
> > > + { 0x0c04, 0x0020, PREFETCH_DEEP | CPRE | CMTLB },
> > > + { 0x0c05, 0x0020, PREFETCH_DEEP | CPRE | CMTLB },
> > > + { 0x0c06, 0x0020, PREFETCH_DEEP | CPRE | CMTLB },
> >
> > [...]
> >
> > Isn't this effectively hard-coding the topology of the SoC in the driver?
> > Wouldn't it better describing higher-level prefetch properties in the DT
> > nodes corresponding to the upstream devices?
>
> Since prefetch data stored in this table represent settings for the
> ACTLR register, and doesn't exactly define the hardware (So in this
> manner prefetch data won't exactly be a part of soc topology ?).

The first two columns of the table are StreamID/Mask pairs, no? How is that
_not_ the SoC topology? I really think it would be better to define some
high-level prefetch properties in the DT binding which can be put on the
master nodes.

> So it seemed apt not to use the device tree for storing the prefetch
> property. Hence we reverted from the DT approach (initial proposal in
> RFC to piggyback on iommus property to store prefetch settings) back to use
> driver for storing this data.
>
> Some drivers use the same approach for storing their platform specific
> data. Examples being
> drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> drivers/soc/qcom/llcc-qcom.c
> These drivers were taken as reference for storing platform specific ACTLR
> data.

I don't know anything about those drivers, but on the SMMU side we already
have ways to describe the topology in the DT and the driver is using them,
so I'm struggling to see the need to add these tables as well.

But as I said before, if Robin and the DT folks prefer this approach,
then I won't get in the way.

Will

2024-03-11 08:43:06

by Bibek Kumar Patro

[permalink] [raw]
Subject: Re: [PATCH v9 4/5] iommu/arm-smmu: add ACTLR data and support for SM8550



On 2/21/2024 6:51 PM, Will Deacon wrote:
> On Wed, Feb 21, 2024 at 02:25:26PM +0530, Bibek Kumar Patro wrote:
>> On 2/13/2024 7:17 PM, Will Deacon wrote:
>>> On Tue, Jan 23, 2024 at 08:15:42PM +0530, Bibek Kumar Patro wrote:
>>>> Add ACTLR data table for SM8550 along with support for
>>>> same including SM8550 specific implementation operations.
>>>>
>>>> Signed-off-by: Bibek Kumar Patro <[email protected]>
>>>> ---
>>>> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 90 ++++++++++++++++++++++
>>>> 1 file changed, 90 insertions(+)
>>>>
>>>> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>>>> index 6004c6d9a7b2..db15b1eade97 100644
>>>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>>>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>>>> @@ -23,6 +23,86 @@
>>>>
>>>> #define CPRE (1 << 1)
>>>> #define CMTLB (1 << 0)
>>>> +#define PREFETCH_SHIFT 8
>>>> +#define PREFETCH_DEFAULT 0
>>>> +#define PREFETCH_SHALLOW (1 << PREFETCH_SHIFT)
>>>> +#define PREFETCH_MODERATE (2 << PREFETCH_SHIFT)
>>>> +#define PREFETCH_DEEP (3 << PREFETCH_SHIFT)
>>>> +#define PREFETCH_SWITCH_GFX (5 << 3)
>>>> +
>>>> +static const struct actlr_config sm8550_apps_actlr_cfg[] = {
>>>> + { 0x18a0, 0x0000, PREFETCH_SHALLOW | CPRE | CMTLB },
>>>> + { 0x18e0, 0x0000, PREFETCH_SHALLOW | CPRE | CMTLB },
>>>> + { 0x0800, 0x0020, PREFETCH_DEFAULT | CMTLB },
>>>> + { 0x1800, 0x00c0, PREFETCH_DEFAULT | CMTLB },
>>>> + { 0x1820, 0x0000, PREFETCH_DEFAULT | CMTLB },
>>>> + { 0x1860, 0x0000, PREFETCH_DEFAULT | CMTLB },
>>>> + { 0x0c01, 0x0020, PREFETCH_DEEP | CPRE | CMTLB },
>>>> + { 0x0c02, 0x0020, PREFETCH_DEEP | CPRE | CMTLB },
>>>> + { 0x0c03, 0x0020, PREFETCH_DEEP | CPRE | CMTLB },
>>>> + { 0x0c04, 0x0020, PREFETCH_DEEP | CPRE | CMTLB },
>>>> + { 0x0c05, 0x0020, PREFETCH_DEEP | CPRE | CMTLB },
>>>> + { 0x0c06, 0x0020, PREFETCH_DEEP | CPRE | CMTLB },
>>>
>>> [...]
>>>
>>> Isn't this effectively hard-coding the topology of the SoC in the driver?
>>> Wouldn't it better describing higher-level prefetch properties in the DT
>>> nodes corresponding to the upstream devices?
>>
>> Since prefetch data stored in this table represent settings for the
>> ACTLR register, and doesn't exactly define the hardware (So in this
>> manner prefetch data won't exactly be a part of soc topology ?).
>
> The first two columns of the table are StreamID/Mask pairs, no? How is that
> _not_ the SoC topology? I really think it would be better to define some
> high-level prefetch properties in the DT binding which can be put on the
> master nodes.
>
>> So it seemed apt not to use the device tree for storing the prefetch
>> property. Hence we reverted from the DT approach (initial proposal in
>> RFC to piggyback on iommus property to store prefetch settings) back to use
>> driver for storing this data.
>>
>> Some drivers use the same approach for storing their platform specific
>> data. Examples being
>> drivers/phy/qualcomm/phy-qcom-qmp-combo.c
>> drivers/soc/qcom/llcc-qcom.c
>> These drivers were taken as reference for storing platform specific ACTLR
>> data.
>
> I don't know anything about those drivers, but on the SMMU side we already
> have ways to describe the topology in the DT and the driver is using them,
> so I'm struggling to see the need to add these tables as well.
>
> But as I said before, if Robin and the DT folks prefer this approach,
> then I won't get in the way.
>

With the driver approach at the current state of patches, it has been
ACKed by DT folks and it seems there has been no concern/objection from
Robin till now.
So can this patch go ahead Will?
Let us know Robin of your opinion as well please.

Thanks & regards,
Bibek

> Will