linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Arnd Bergmann <arnd@kernel.org>
To: Min Li <min.li.xe@renesas.com>
Cc: Derek Kiernan <derek.kiernan@xilinx.com>,
	Dragan Cvetic <dragan.cvetic@xilinx.com>,
	Arnd Bergmann <arnd@arndb.de>,
	gregkh <gregkh@linuxfoundation.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Networking <netdev@vger.kernel.org>,
	Richard Cochran <richardcochran@gmail.com>
Subject: Re: [PATCH net-next] misc: Add Renesas Synchronization Management Unit (SMU) support
Date: Fri, 12 Feb 2021 20:50:39 +0100	[thread overview]
Message-ID: <CAK8P3a3k5dAF=X3_NrYAAp5gPJ_uvF3XfmC4rKz0oGTrGRriCw@mail.gmail.com> (raw)
In-Reply-To: <OSBPR01MB4773B22EA094A362DD807F83BA8B9@OSBPR01MB4773.jpnprd01.prod.outlook.com>

On Fri, Feb 12, 2021 at 5:19 PM Min Li <min.li.xe@renesas.com> wrote:
>
> >
> > Ah, so if this is for a PTP related driver, it should probably be integrated into
> > the PTP subsystem rather than being a separate class.
> >
>
> I was trying to add these functions to PHC subsystem but was not accepted because the functions
> are specific to Renesas device and there is no place for those functions in PHC driver.

It would be useful to explain that in the patch description and link
to the original
discussion there. What exactly was the objection?

> > > > This tells me that you got the abstraction the wrong way: the common
> > > > files should not need to know anything about the specific
> > implementations.
> > > >
> > > > Instead, these should be in separate modules that call exported
> > > > functions from the common code.
> > > >
> > > >
> > >
> > > I got what you mean. But so far it only supports small set of
> > > functions, which is why I don't feet it is worth the effort to over abstract
> > things.
> >
> > Then maybe pick one of the two hardware variants and drop the abstraction
> > you have. You can then add more features before you add a proper
> > abstraction layer and then the second driver.
> >
>
> If I come up with a new file and move all the abstraction code there,
> does that work?

I think so, but it's more important to figure out a good user space
interface first. The ioctl interfaces should be written on a higher-level
abstraction, to ensure they can work with any hardware implementation
and are not specific to Renesas devices.

Can you describe on an abstract level how a user would use the
character device, and what they achieve by that?

       Arnd

  reply	other threads:[~2021-02-12 19:51 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-11  3:03 [PATCH net-next] misc: Add Renesas Synchronization Management Unit (SMU) support min.li.xe
2021-02-11  5:47 ` kernel test robot
2021-02-11  7:06 ` Greg KH
2021-02-11 12:54 ` Greg KH
2021-02-11 13:29 ` Arnd Bergmann
     [not found]   ` <OSBPR01MB47732017A97D5C911C4528F0BA8B9@OSBPR01MB4773.jpnprd01.prod.outlook.com>
2021-02-12  9:24     ` Arnd Bergmann
2021-02-12 16:16       ` Min Li
2021-02-12 19:50         ` Arnd Bergmann [this message]
2021-02-16 17:10           ` Min Li
2021-02-16 20:44             ` Arnd Bergmann
2021-02-16 22:14               ` Min Li
2021-02-17 15:56                 ` Arnd Bergmann
     [not found]                   ` <OSBPR01MB477394546AE3BC1F186FC0E9BA869@OSBPR01MB4773.jpnprd01.prod.outlook.com>
2021-02-17 21:30                     ` Arnd Bergmann
2021-02-17 23:07                       ` Jakub Kicinski
2021-02-18  3:28                       ` Min Li
2021-02-18 10:50                         ` Arnd Bergmann
2021-02-18 16:14                           ` Min Li
2021-02-18 23:03                           ` FW: " Min Li
2021-02-22 16:21                           ` Min Li

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='CAK8P3a3k5dAF=X3_NrYAAp5gPJ_uvF3XfmC4rKz0oGTrGRriCw@mail.gmail.com' \
    --to=arnd@kernel.org \
    --cc=arnd@arndb.de \
    --cc=derek.kiernan@xilinx.com \
    --cc=dragan.cvetic@xilinx.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=min.li.xe@renesas.com \
    --cc=netdev@vger.kernel.org \
    --cc=richardcochran@gmail.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).