[PATCH] staging: vchiq_arm: Add 36-bit address support

[Stefan Wahren]

Hi Mwesigwa,

additional to Nicolas' comments.

Am 15.10.21 um 10:19 schrieb nicolas saenz julienne:
> Hi Mwesigwa,
>
> On Thu, 2021-10-14 at 18:32 -0400, Mwesigwa Guma wrote:
>> Cc: Nicolas Saenz Julienne <nsaenz at kernel.org>
>> Cc: Greg Kroah-Hartman <gregkh at linuxfoundation.org>
>> Cc: Stefan Wahren <stefan.wahren at i2se.com>
>> Cc: Ojaswin Mujoo <ojaswin98 at gmail.com>
>> Cc: Dan Carpenter <dan.carpenter at oracle.com>
>> Cc: Amarjargal Gundjalam <amarjargal16 at gmail.com>
>> Cc: Phil Elwell <phil at raspberrypi.com>
>> Cc: bcm-kernel-feedback-list at broadcom.com
>> Cc: linux-rpi-kernel at lists.infradead.org 
>> Cc: linux-arm-kernel at lists.infradead.org 
>> Cc: linux-staging at lists.linux.dev 
>> Cc: fedora-rpi at googlegroups.com
>> Cc: Joel Savitz <jsavitz at redhat.com>
>> Cc: Chukpozohn Toe <ctoe at redhat.com>
>> Cc: Clark Williams <clark at redhat.com>
>>
>> This is a forward port of Phil Elwell's commit from the Raspberry Pi
>> Linux fork described as follows [1]:
>>
>>     Conditional on a new compatible string, change the pagelist encoding
>>     such that the top 24 bits are the pfn, leaving 8 bits for run length
>>     (-1), giving a 36-bit address range.
>>     
>>     Manage the split between addresses for the VPU and addresses for the
>>     40-bit DMA controller with a dedicated DMA device pointer that on non-
>>     BCM2711 platforms is the same as the main VCHIQ device. This allows
>>     the VCHIQ node to stay in the usual place in the DT.
From my understanding this also requires changes to the DMA driver (40
bit support) and the BCM2711 dts files, which are also not upstream yet
(the DMA changes not the files itself).
>>
>> This commit enables VCHIQ device access on a Raspberry Pi 4B running the 
>> mainline Linux kernel.
>>
>> Tested on Fedora Linux running on a Raspberry Pi 4B.
>>
>> [1]: https://github.com/raspberrypi/linux/commit/97268fd23eb8d08dc74eac5e3fd697303de26610
>>
>> Signed-off-by: Mwesigwa Guma <mguma at redhat.com>
> I see a lot happening on this patch. You're:
>
>  - Registering a number of child devices that don't seem to exist upstream
>    ('vcsm-cma', 'bcm2835-codec', and 'bcm2825-isp').
>  - Updating vchiq_register_child(). 
>  - Adding brcm,bcm2711-vchiq's compatible string (no dt bindings? see
>    Documentation/devicetree/bindings/soc/bcm/brcm,bcm2835-vchiq.txt).
In order to get this part accepted, i send a series to convert the
dt-binding to YAML format.
>  - Looking for 'brcm,bcm2711-dma' in the devicetree.
>  - Using 'brcm,bcm2711-dma's' device node to re-generate the page lists.
>
> Each one of these should at least be a separate patch, which proper
> justification[1]. You can't just take downstream fixes, rebase them and send
> them upstream. You really have to own them, undestand what's happening and
> repurpose everything so it's up to standard even if it means diverging from
> what downstream is doing.

AFAIU it's preferred to get this driver out of staging (take a look at
the TODO file) before implementing new features.

Best regards

>
> Regards,
> Nicolas
>
> [1] Have a look at Documentation/process/submitting-patches.rst.
>