[2/5] dmaengine: qcom: gpi: document preferred SM6350 binding

  Devices with ee offset of 0x10000 should rather bind with SM6350
compatible, so the list will not unnecessarily grow for compatible
devices.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 drivers/dma/qcom/gpi.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
  

> Devices with ee offset of 0x10000 should rather bind with SM6350
> compatible, so the list will not unnecessarily grow for compatible
> devices.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  drivers/dma/qcom/gpi.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/dma/qcom/gpi.c b/drivers/dma/qcom/gpi.c
> index f8e19e6e6117..061add832295 100644
> --- a/drivers/dma/qcom/gpi.c
> +++ b/drivers/dma/qcom/gpi.c
> @@ -2286,13 +2286,14 @@ static int gpi_probe(struct platform_device *pdev)
>  }
>  
>  static const struct of_device_id gpi_of_match[] = {
> -	{ .compatible = "qcom,sc7280-gpi-dma", .data = (void *)0x10000 },
>  	{ .compatible = "qcom,sdm845-gpi-dma", .data = (void *)0x0 },
>  	{ .compatible = "qcom,sm6350-gpi-dma", .data = (void *)0x10000 },
>  	/*
> -	 * Deprecated, devices with ee_offset = 0 should use sdm845-gpi-dma as
> -	 * fallback and not need their own entries here.

This comment is from the dependency series [1]. Why would we need to add it just
to remove it here? I was not notified that the dependency was applied anywhere
(except as a base for other series) so it's not set in stone. Let's just drop
the original patch that this comment originates from to prevent needlessly
adding and removing the same lines at once.

[1] https://lore.kernel.org/linux-arm-msm/20221007213640.85469-4-mailingradian@gmail.com/

> +	 * Do not grow the list for compatible devices. Instead use
> +	 * qcom,sdm845-gpi-dma (for ee_offset = 0x0) or qcom,sm6350-gpi-dma
> +	 * (for ee_offset = 0x10000).
>  	 */
> +	{ .compatible = "qcom,sc7280-gpi-dma", .data = (void *)0x10000 },
>  	{ .compatible = "qcom,sm8150-gpi-dma", .data = (void *)0x0 },
>  	{ .compatible = "qcom,sm8250-gpi-dma", .data = (void *)0x0 },
>  	{ .compatible = "qcom,sm8350-gpi-dma", .data = (void *)0x10000 },
> -- 
> 2.34.1
>
  

On 17/10/2022 17:23, Richard Acayan wrote:
>> Devices with ee offset of 0x10000 should rather bind with SM6350
>> compatible, so the list will not unnecessarily grow for compatible
>> devices.
>>
>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> ---
>>  drivers/dma/qcom/gpi.c | 7 ++++---
>>  1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/dma/qcom/gpi.c b/drivers/dma/qcom/gpi.c
>> index f8e19e6e6117..061add832295 100644
>> --- a/drivers/dma/qcom/gpi.c
>> +++ b/drivers/dma/qcom/gpi.c
>> @@ -2286,13 +2286,14 @@ static int gpi_probe(struct platform_device *pdev)
>>  }
>>  
>>  static const struct of_device_id gpi_of_match[] = {
>> -	{ .compatible = "qcom,sc7280-gpi-dma", .data = (void *)0x10000 },
>>  	{ .compatible = "qcom,sdm845-gpi-dma", .data = (void *)0x0 },
>>  	{ .compatible = "qcom,sm6350-gpi-dma", .data = (void *)0x10000 },
>>  	/*
>> -	 * Deprecated, devices with ee_offset = 0 should use sdm845-gpi-dma as
>> -	 * fallback and not need their own entries here.
> 
> This comment is from the dependency series [1]. Why would we need to add it just
> to remove it here? I was not notified that the dependency was applied anywhere
> (except as a base for other series) so it's not set in stone. Let's just drop
> the original patch that this comment originates from to prevent needlessly
> adding and removing the same lines at once.

I don't remove the comment, I re-phrase it to be better suited for final
code.

Best regards,
Krzysztof
  

> On 17/10/2022 17:23, Richard Acayan wrote:
>>> Devices with ee offset of 0x10000 should rather bind with SM6350
>>> compatible, so the list will not unnecessarily grow for compatible
>>> devices.
>>>
>>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>> ---
>>>  drivers/dma/qcom/gpi.c | 7 ++++---
>>>  1 file changed, 4 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/dma/qcom/gpi.c b/drivers/dma/qcom/gpi.c
>>> index f8e19e6e6117..061add832295 100644
>>> --- a/drivers/dma/qcom/gpi.c
>>> +++ b/drivers/dma/qcom/gpi.c
>>> @@ -2286,13 +2286,14 @@ static int gpi_probe(struct platform_device *pdev)
>>>  }
>>>  
>>>  static const struct of_device_id gpi_of_match[] = {
>>> -	{ .compatible = "qcom,sc7280-gpi-dma", .data = (void *)0x10000 },
>>>  	{ .compatible = "qcom,sdm845-gpi-dma", .data = (void *)0x0 },
>>>  	{ .compatible = "qcom,sm6350-gpi-dma", .data = (void *)0x10000 },
>>>  	/*
>>> -	 * Deprecated, devices with ee_offset = 0 should use sdm845-gpi-dma as
>>> -	 * fallback and not need their own entries here.
>> 
>> This comment is from the dependency series [1]. Why would we need to add it just
>> to remove it here? I was not notified that the dependency was applied anywhere
>> (except as a base for other series) so it's not set in stone. Let's just drop
>> the original patch that this comment originates from to prevent needlessly
>> adding and removing the same lines at once.
> 
> I don't remove the comment, I re-phrase it to be better suited for final
> code.

Yes, I didn't word that exactly right. I still think the patch that adds this is
now useless if it's just going to be replaced. Do you think I should keep the
patch that this comment originates from, even though we already know exactly how
its substantial contents will be replaced?

We can't modify history and drop commits on kernel trees, but I can still send
a v6 series that drops the original comment.

> 
> Best regards,
> Krzysztof
>
  

On 17/10/2022 18:00, Richard Acayan wrote:
>> On 17/10/2022 17:23, Richard Acayan wrote:
>>>> Devices with ee offset of 0x10000 should rather bind with SM6350
>>>> compatible, so the list will not unnecessarily grow for compatible
>>>> devices.
>>>>
>>>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>>> ---
>>>>  drivers/dma/qcom/gpi.c | 7 ++++---
>>>>  1 file changed, 4 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/dma/qcom/gpi.c b/drivers/dma/qcom/gpi.c
>>>> index f8e19e6e6117..061add832295 100644
>>>> --- a/drivers/dma/qcom/gpi.c
>>>> +++ b/drivers/dma/qcom/gpi.c
>>>> @@ -2286,13 +2286,14 @@ static int gpi_probe(struct platform_device *pdev)
>>>>  }
>>>>  
>>>>  static const struct of_device_id gpi_of_match[] = {
>>>> -	{ .compatible = "qcom,sc7280-gpi-dma", .data = (void *)0x10000 },
>>>>  	{ .compatible = "qcom,sdm845-gpi-dma", .data = (void *)0x0 },
>>>>  	{ .compatible = "qcom,sm6350-gpi-dma", .data = (void *)0x10000 },
>>>>  	/*
>>>> -	 * Deprecated, devices with ee_offset = 0 should use sdm845-gpi-dma as
>>>> -	 * fallback and not need their own entries here.
>>>
>>> This comment is from the dependency series [1]. Why would we need to add it just
>>> to remove it here? I was not notified that the dependency was applied anywhere
>>> (except as a base for other series) so it's not set in stone. Let's just drop
>>> the original patch that this comment originates from to prevent needlessly
>>> adding and removing the same lines at once.
>>
>> I don't remove the comment, I re-phrase it to be better suited for final
>> code.
> 
> Yes, I didn't word that exactly right. I still think the patch that adds this is
> now useless if it's just going to be replaced. Do you think I should keep the
> patch that this comment originates from, even though we already know exactly how
> its substantial contents will be replaced?
> 
> We can't modify history and drop commits on kernel trees, but I can still send
> a v6 series that drops the original comment.

Sure. You can make it then:

	 * Do not grow the list for compatible devices. Instead use
	 * qcom,sdm845-gpi-dma (for ee_offset = 0x0).

And my patch will just change one line.

We can also keep it like:

	 * Do not grow the list for compatible devices. Instead use
	 * proper fallback compatibles.

Best regards,
Krzysztof
  

> On 17/10/2022 18:00, Richard Acayan wrote:
>>> On 17/10/2022 17:23, Richard Acayan wrote:
>>>>> Devices with ee offset of 0x10000 should rather bind with SM6350
>>>>> compatible, so the list will not unnecessarily grow for compatible
>>>>> devices.
>>>>>
>>>>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>>>> ---
>>>>>  drivers/dma/qcom/gpi.c | 7 ++++---
>>>>>  1 file changed, 4 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/drivers/dma/qcom/gpi.c b/drivers/dma/qcom/gpi.c
>>>>> index f8e19e6e6117..061add832295 100644
>>>>> --- a/drivers/dma/qcom/gpi.c
>>>>> +++ b/drivers/dma/qcom/gpi.c
>>>>> @@ -2286,13 +2286,14 @@ static int gpi_probe(struct platform_device *pdev)
>>>>>  }
>>>>>  
>>>>>  static const struct of_device_id gpi_of_match[] = {
>>>>> -	{ .compatible = "qcom,sc7280-gpi-dma", .data = (void *)0x10000 },
>>>>>  	{ .compatible = "qcom,sdm845-gpi-dma", .data = (void *)0x0 },
>>>>>  	{ .compatible = "qcom,sm6350-gpi-dma", .data = (void *)0x10000 },
>>>>>  	/*
>>>>> -	 * Deprecated, devices with ee_offset = 0 should use sdm845-gpi-dma as
>>>>> -	 * fallback and not need their own entries here.
>>>>
>>>> This comment is from the dependency series [1]. Why would we need to add it just
>>>> to remove it here? I was not notified that the dependency was applied anywhere
>>>> (except as a base for other series) so it's not set in stone. Let's just drop
>>>> the original patch that this comment originates from to prevent needlessly
>>>> adding and removing the same lines at once.
>>>
>>> I don't remove the comment, I re-phrase it to be better suited for final
>>> code.
>> 
>> Yes, I didn't word that exactly right. I still think the patch that adds this is
>> now useless if it's just going to be replaced. Do you think I should keep the
>> patch that this comment originates from, even though we already know exactly how
>> its substantial contents will be replaced?
>> 
>> We can't modify history and drop commits on kernel trees, but I can still send
>> a v6 series that drops the original comment.
> 
> Sure. You can make it then:
> 
> 	 * Do not grow the list for compatible devices. Instead use
> 	 * qcom,sdm845-gpi-dma (for ee_offset = 0x0).

If you don't want me to drop the original patch completely, then there is no
need to make any changes at all to the driver patches IMHO. I originally thought
we only needed one patch for the driver (yours) but you seem to have a really
good reason not to drop the original patch. Sorry for asking.

I guess you can add this if you want:

Acked-by: Richard Acayan <mailingradian@gmail.com>

> 
> And my patch will just change one line.
> 
> We can also keep it like:
> 
> 	 * Do not grow the list for compatible devices. Instead use
> 	 * proper fallback compatibles.
> 
> Best regards,
> Krzysztof
>