Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   "Y.b. Lu" <yangbo.lu@nxp.com>
To:     Ioana Ciocoi Radulescu <ruxandra.radulescu@nxp.com>,
        Andrew Lunn <andrew@lunn.ch>
CC:     "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "devel@driverdev.osuosl.org" <devel@driverdev.osuosl.org>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        Richard Cochran <richardcochran@gmail.com>,
        "David S . Miller" <davem@davemloft.net>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Subject: RE: [PATCH 1/2] net: dpaa2: move DPAA2 PTP driver out of staging/
Thread-Topic: [PATCH 1/2] net: dpaa2: move DPAA2 PTP driver out of staging/
Thread-Index: AQHUVlODYLzUr6TXRkCk3Aso2dGK1KUEHlyAgAEqd6CAADRngIABD/MA
Date:   Sat, 29 Sep 2018 03:06:19 +0000
Message-ID: <VI1PR0401MB22372C3094F743054679C53CF8ED0@VI1PR0401MB2237.eurprd04.prod.outlook.com>
References: <20180927111228.46118-1-yangbo.lu@nxp.com>
 <20180927132507.GB23375@lunn.ch>
 <VI1PR0401MB223737CACE4E4064AE8325A4F8EC0@VI1PR0401MB2237.eurprd04.prod.outlook.com>
 <DBXPR04MB3490DB9A1EF2D814568843B94EC0@DBXPR04MB349.eurprd04.prod.outlook.com>
In-Reply-To: <DBXPR04MB3490DB9A1EF2D814568843B94EC0@DBXPR04MB349.eurprd04.prod.outlook.com>
Accept-Language: en-US
Content-Language: en-US
received-spf: None (protection.outlook.com: nxp.com does not designate
 permitted sender hosts)
spamdiagnosticoutput: 1:99
spamdiagnosticmetadata: NSPM
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 731109ae-a199-4570-ff66-08d625b8876f
X-MS-Exchange-CrossTenant-originalarrivaltime: 29 Sep 2018 03:06:19.4884
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR0401MB2334
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Ioana,

> -----Original Message-----
> From: Ioana Ciocoi Radulescu
> Sent: Friday, September 28, 2018 6:21 PM
> To: Y.b. Lu <yangbo.lu@nxp.com>; Andrew Lunn <andrew@lunn.ch>
> Cc: linux-kernel@vger.kernel.org; devel@driverdev.osuosl.org;
> netdev@vger.kernel.org; Richard Cochran <richardcochran@gmail.com>;
> David S . Miller <davem@davemloft.net>; Greg Kroah-Hartman
> <gregkh@linuxfoundation.org>
> Subject: RE: [PATCH 1/2] net: dpaa2: move DPAA2 PTP driver out of staging=
/
>=20
> > -----Original Message-----
> > From: Y.b. Lu
> > Sent: Friday, September 28, 2018 11:04 AM
> > To: Andrew Lunn <andrew@lunn.ch>
> > Cc: linux-kernel@vger.kernel.org; devel@driverdev.osuosl.org;
> > netdev@vger.kernel.org; Richard Cochran <richardcochran@gmail.com>;
> > David S . Miller <davem@davemloft.net>; Ioana Ciocoi Radulescu
> > <ruxandra.radulescu@nxp.com>; Greg Kroah-Hartman
> > <gregkh@linuxfoundation.org>
> > Subject: RE: [PATCH 1/2] net: dpaa2: move DPAA2 PTP driver out of
> > staging/
> >
[...]
> > >
> > > It seems like there is a lot of code in dprtc.c which is unused.
> > > rtc.c does
> > nothing
> > > with interrupts for example. Do you plan to make use of this extra
> > > code? Or can it be removed leaving just what is needed?
> >
> > [Y.b. Lu] Currently the ptp/rtc driver is not full-featured. The extra
> > code is being planed to be used.
>=20
> Are there any interrupts associated to the real time clock module that wi=
ll
> actually be used by the driver? Also, I don't think the create/destroy fu=
nctions
> are meant to be used by the PTP kernel driver, even though MC exposes the
> APIs for them.
>=20
> Generally speaking, I think it's better to remove unused code from the cu=
rrent
> driver and re-add it along with the feature actually using it.

[Y.b. Lu] Yes. We need to implement these interrupts to support ptp_clock_e=
vent() of common ptp_clock driver.
This is mainly to support 1588 timer external signals.
I get your point, and will remove unused code before using them.

>=20
> >
> > >
> > > struct dprtc_cmd_get_irq - Putting pad at the beginning of a struct
> > > seems
> > very
> > > odd. And it is not the only example.
> >
> > [Y.b. Lu] This should depended on MC firmware and APIs I think. Once
> > the MC improves this, the APIs could be updated to fix this.
>=20
> These structures map the command format expected by the MC firmware. I
> agree that some of the command layouts are less than inspired, but I'm no=
t
> sure we can expect MC to "improve" them without a good reason, as this
> would break backward compatibility.
>=20
> I also want to bring up the question of where the dpaa2 ptp driver should=
 be
> located. The qoriq_ptp driver (which targets previous gen Freescale/NXP
> architectures) is located in drivers/ptp. I'm not sure if the dpaa2 ptp d=
river
> should be moved there as well or it's better suited for the currently pro=
posed
> location.

[Y.b. Lu] Actually the ptp timer is to provide hw timestamping support for =
ethernet.
Ptp clock driver together with ethernet hw timestamping driver provide the =
method to support 1588 software application.
It's ok to put ptp clock driver near to ethernet driver. And most ptp clock=
 drivers in kernel are together with ethernet driver.
You may see there are gianfar_ptp and dpaa_ptp before. Considering they cou=
ld reuse the code, I created the ptp_qoriq for them to use the one driver.

>=20
> Thanks,
> Ioana