Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <1613012611-8489-1-git-send-email-min.li.xe@renesas.com>
 <CAK8P3a3YhAGEfrvmi4YhhnG_3uWZuQi0ChS=0Cu9c4XCf5oGdw@mail.gmail.com>
 <OSBPR01MB47732017A97D5C911C4528F0BA8B9@OSBPR01MB4773.jpnprd01.prod.outlook.com>
 <CAK8P3a2KDO4HutsXNJzjmRJTvW1QW4Kt8H7U53_QqpmgvZtd3A@mail.gmail.com> <OSBPR01MB4773B22EA094A362DD807F83BA8B9@OSBPR01MB4773.jpnprd01.prod.outlook.com>
In-Reply-To: <OSBPR01MB4773B22EA094A362DD807F83BA8B9@OSBPR01MB4773.jpnprd01.prod.outlook.com>
From:   Arnd Bergmann <arnd@kernel.org>
Date:   Fri, 12 Feb 2021 20:50:39 +0100
Message-ID: <CAK8P3a3k5dAF=X3_NrYAAp5gPJ_uvF3XfmC4rKz0oGTrGRriCw@mail.gmail.com>
Subject: Re: [PATCH net-next] misc: Add Renesas Synchronization Management
 Unit (SMU) support
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>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

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