From mboxrd@z Thu Jan  1 00:00:00 1970
From: <Nisar.Sayed@microchip.com>
Subject: RE: [PATCH net-next] microchipT1phy: Add driver for Microchip
 LAN87XX T1 PHYs
Date: Thu, 26 Apr 2018 17:24:41 +0000
Message-ID: <CE371C1263339941885964188A0225FA3B1132@CHN-SV-EXMX03.mchp-main.com>
References: <20180425184944.24939-1-Nisar.Sayed@microchip.com>
 <accc8a8b-133f-7f68-8a64-147cd981dab3@gmail.com>
 <CE371C1263339941885964188A0225FA3B0CB2@CHN-SV-EXMX03.mchp-main.com>
 <20180426122743.GB13467@lunn.ch>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 8BIT
Cc: <f.fainelli@gmail.com>, <davem@davemloft.net>,
        <UNGLinuxDriver@microchip.com>, <netdev@vger.kernel.org>
To: <andrew@lunn.ch>
Return-path: <netdev-owner@vger.kernel.org>
Received: from esa5.microchip.iphmx.com ([216.71.150.166]:42337 "EHLO
        esa5.microchip.iphmx.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1756926AbeDZRY7 (ORCPT
        <rfc822;netdev@vger.kernel.org>); Thu, 26 Apr 2018 13:24:59 -0400
In-Reply-To: <20180426122743.GB13467@lunn.ch>
Content-Language: en-US
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>

Hi Andrew,

> > Fine, will change the filename.
> 
> > The reason for moving to separate file is that we have a series of
> > T1 standard PHYs, which support cable diagnostics, signal quality
> > indicator(SQI) and sleep and wakeup (TC10) support etc. we planned to
> > keep all T1 standard PHYs separate to support additional features
> > supported by these PHYs.
> 
> Is there anything shared with the other microchip PHYs? If there is potential
> for code sharing, you should do it.

Yes, there will be no code sharing between existing microchip PHYs and the newly getting added T1 phys.

> 
> > > > + */
> > > > +#ifndef _MICROCHIPT1PHY_H_
> > > > +#define _MICROCHIPT1PHY_H_
> > > > +
> > > > +/* Interrupt Source Register */
> > > > +#define LAN87XX_INTERRUPT_SOURCE                (0x18)
> > > > +
> > > > +/* Interrupt Mask Register */
> > > > +#define LAN87XX_INTERRUPT_MASK                  (0x19)
> > > > +#define LAN87XX_MASK_LINK_UP                    (0x0004)
> > > > +#define LAN87XX_MASK_LINK_DOWN                  (0x0002)
> > >
> > > What's the point of that header file if all definitions are consumed
> > > by the same driver?
> > >
> >
> > We have planned a series of patches where we planned to use this further.
> 
> Are you adding multiple files which share the header? If not, just add the
> defines to the C code.
> 
>     Andrew

We have a plan, I think as you suggested better to go with defines in C codes itself now.
Maybe we can create/move during future submissions.

- Nisar