[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <edd6f6ad-f06f-5871-a3bb-da18e114c135@i2se.com>
Date: Sun, 20 Jun 2021 15:28:43 +0200
From: Stefan Wahren <stefan.wahren@...e.com>
To: Ojaswin Mujoo <ojaswin98@...il.com>, nsaenz@...nel.org
Cc: gregkh@...uxfoundation.org, arnd@...db.de,
dan.carpenter@...cle.com, phil@...pberrypi.com,
bcm-kernel-feedback-list@...adcom.com,
linux-arm-kernel@...ts.infradead.org,
linux-staging@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 0/5] vchiq: Patch to separate platform and cdev code
Hi Ojaswin,
Am 20.06.21 um 14:55 schrieb Ojaswin Mujoo:
> Hello,
>
> This patchset adderesses the TODO item number 10 specified at:
>
> drivers/staging/vc04-services/interface/TODO
>
> For reference, the task is:
>
> 10) Reorganize file structure: Move char driver to it's own file and join
> both platform files
>
> The cdev is defined alongside with the platform code in vchiq_arm.c. It
> would be nice to completely decouple it from the actual core code. For
> instance to be able to use bcm2835-audio without having /dev/vchiq created.
> One could argue it's better for security reasons or general cleanliness. It
> could even be interesting to create two different kernel modules, something
> the likes of vchiq-core.ko and vchiq-dev.ko. This would also ease the
> upstreaming process.
>
> As Stefan suggested in the last revision, I have split the commits into
> more finer parts for ease of readability and maintainability. I have
> also added 2 more patches to define a KConfig entry for vchiq cdev, and
> to merge the code in vchiq_2835_arm.c to vchiq_arm.c
>
> A summary of the patches is now as follows:
>
> - Patch 1: Move cdev init code into a function
> - Patch 2: Shift some devlarations from vchiq_arm.c to vchiq_arm.h for
> sharing
> - Patch 3: Move vchiq cdev init code from vchiq_arm.c into vchiq_dev.c
> - Patch 4: Decouple cdev code by defining a Kconfig entry to allow
> optional compilation of it.
> - Patch 5: Merge code in vchiq_2835_arm.c to vchiq_arm.c
>
> (More details can be found in the commit messages)
>
>
> NOTE: This patchset is built against the raspberry pi mainline kernel at
> https://github.com/raspberrypi/linux/blob/rpi-5.10.y, and has been
> tested on Raspberry Pi 3B+
please don't do this, because it's a waste of time. Greg can only apply
patches against the mainline kernel and the patches must be tested with
the mainline kernel. Additionally i've sent a lot of patches recently
which are not applied against the vendor tree.
>
> At this point, I have some questions and ideas and would like to hear your
> thoughts and suggestions on them:
>
> 1. So as mentioned, I have built this against the raspberry pi kernel,
> since I was not able to figure out a way to build the vanilla
> mainline kernel for Raspberry Pi. However, I understand that since
> this will be applied to the mainline, I need to make sure it is
> consistent with it.
Can you please describe the issue in detail?
Or try this older guide [1]
Best regards
Stefan Wahren
[1] - https://gist.github.com/lategoodbye/c7317a42bf7f9c07f5a91baed8c68f75
>
> Hence to confirm that, I tried to "git am" this patchset to the
> mainline kernel but there are some merge conflicts in doing so. I
> have an idea how to resolve most of them except the following:
>
> - The mainline vchiq_arm.c differs from the one in rapberry pi
> mainline which caused conflict in Patch 3.
>
> I'm not sure which vchiq_arm.c to treat as the base for my patches.
> The one in mainline? or the one in raspberry pi's git tree?
>
>
> 2. This question is more related to the next set of patches I'm
> planning to submit. So the last thing left in this TODO is to
> completely decouple vchiq platform and cdev code into 2 separate
> modules and I am planning to do that in a different patchset.
>
> The approach I have in mind is to start by using EXPORT_SYMBOL to
> export all the functions (and accessor functions for variables like
> g_state) that would be required for cdev init. Majority of these
> would be exported from vchiq_arm.c and vchiq_core.c, and will then be
> used in vchiq-dev.ko. Is this the right way to approach this?
>
> Thank you in advance for looking into this and best regards!
> Ojaswin
>
>
> Ojaswin Mujoo (5):
> staging: vchiq: Refactor vchiq cdev code
> staging: vchiq: Move certain declarations to vchiq_arm.h
> staging: vchiq: Move vchiq char driver to its own file
> staging: vchiq: Make creation of vchiq cdev optional
> staging: vchiq: Combine vchiq platform code into single file
>
> arch/arm/configs/bcm2709_defconfig | 1 +
> arch/arm/configs/bcm2711_defconfig | 1 +
> arch/arm/configs/bcmrpi_defconfig | 1 +
> drivers/staging/vc04_services/Kconfig | 10 +
> drivers/staging/vc04_services/Makefile | 5 +-
> .../interface/vchiq_arm/vchiq_2835_arm.c | 651 -----
> .../interface/vchiq_arm/vchiq_arm.c | 2477 ++++++-----------
> .../interface/vchiq_arm/vchiq_arm.h | 79 +
> .../interface/vchiq_arm/vchiq_dev.c | 1488 ++++++++++
> 9 files changed, 2402 insertions(+), 2311 deletions(-)
> delete mode 100644 drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
> create mode 100644 drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c
>
Powered by blists - more mailing lists