linux-staging.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
From: Ojaswin Mujoo <ojaswin98@gmail.com>
To: Stefan Wahren <stefan.wahren@i2se.com>
Cc: nsaenz@kernel.org, gregkh@linuxfoundation.org, arnd@arndb.de,
	dan.carpenter@oracle.com, phil@raspberrypi.com,
	bcm-kernel-feedback-list@broadcom.com,
	linux-arm-kernel@lists.infradead.org,
	linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 0/5] vchiq: Patch to separate platform and cdev code
Date: Mon, 21 Jun 2021 19:41:58 +0530	[thread overview]
Message-ID: <20210621141158.GA186639@ojas> (raw)
In-Reply-To: <edd6f6ad-f06f-5871-a3bb-da18e114c135@i2se.com>

On Sun, Jun 20, 2021 at 03:28:43PM +0200, Stefan Wahren wrote:
Hello Stefan!

> 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.
Understood Stefan, I'll keep that in mind going forward.
> >
> > 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]
Thanks for the guide, I tried this out (along with some other things) but 
I'm not able to get the Pi to boot. I'm using a headless setup with
Wifi+SSH, due to lack of hardware, and I'm not able to SSH/ping when using
mainline. This works correctly with the downstream kernel + Raspbian.
(Maybe because of missing drivers and/or config options).

I'm still looking into this but a I'm a bit in the dark till I can get my
hands on a UART to USB cable although they are difficult to get
around here. I'm also trying to maybe figure out if I can somehow use my
Arduino to capture the serial output. 

May I ask what setup you are using for kernel development on pi? 
> 
> Best regards
> Stefan Wahren
> 
> [1] - https://gist.github.com/lategoodbye/c7317a42bf7f9c07f5a91baed8c68f75
> 
Thank you again for the help!
Ojaswin
> >
> >    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
> >
> 

      reply	other threads:[~2021-06-21 14:12 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-20 12:55 [PATCH v2 0/5] vchiq: Patch to separate platform and cdev code Ojaswin Mujoo
2021-06-20 12:55 ` [PATCH v2 1/5] staging: vchiq: Refactor vchiq " Ojaswin Mujoo
2021-06-21  8:21   ` Dan Carpenter
2021-06-21 14:22     ` Ojaswin Mujoo
2021-06-20 12:56 ` [PATCH v2 2/5] staging: vchiq: Move certain declarations to vchiq_arm.h Ojaswin Mujoo
2021-06-20 12:56 ` [PATCH v2 3/5] staging: vchiq: Move vchiq char driver to its own file Ojaswin Mujoo
2021-06-21  9:56   ` Dan Carpenter
2021-06-21 14:28     ` Ojaswin Mujoo
2021-06-20 12:57 ` [PATCH v2 4/5] staging: vchiq: Make creation of vchiq cdev optional Ojaswin Mujoo
2021-06-20 12:57 ` [PATCH v2 5/5] staging: vchiq: Combine vchiq platform code into single file Ojaswin Mujoo
2021-06-20 13:28 ` [PATCH v2 0/5] vchiq: Patch to separate platform and cdev code Stefan Wahren
2021-06-21 14:11   ` Ojaswin Mujoo [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210621141158.GA186639@ojas \
    --to=ojaswin98@gmail.com \
    --cc=arnd@arndb.de \
    --cc=bcm-kernel-feedback-list@broadcom.com \
    --cc=dan.carpenter@oracle.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-staging@lists.linux.dev \
    --cc=nsaenz@kernel.org \
    --cc=phil@raspberrypi.com \
    --cc=stefan.wahren@i2se.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).