Date: Thu, 25 Dec 2014 10:03:02 +0800
From: Peter Chen <peter.chen@freescale.com>
To: Stefan Agner <stefan@agner.ch>
CC: Sanchayan Maity <maitysanchayan@gmail.com>, <gregkh@linuxfoundation.org>,
        <linux-usb@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 1/3] usb: chipidea: Add identification registers access
 APIs
Message-ID: <20141225020259.GA5077@shlinux2>
References: <cover.1418981438.git.maitysanchayan@gmail.com>
 <72124df17c0f40f8a7ae02faa9664b9d51743584.1418981438.git.maitysanchayan@gmail.com>
 <4f5efe169ed3ab16ffcbee52dd6b7661@agner.ch>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <4f5efe169ed3ab16ffcbee52dd6b7661@agner.ch>
User-Agent: Mutt/1.5.21 (2010-09-15)
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 25 Dec 2014 03:11:54.8560
 (UTC)
X-MS-Exchange-CrossTenant-Id: 710a03f5-10f6-4d38-9ff4-a80b81da590d
X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=710a03f5-10f6-4d38-9ff4-a80b81da590d;Ip=[192.88.168.50]
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BLUPR03MB214
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Wed, Dec 24, 2014 at 05:00:05PM +0100, Stefan Agner wrote:
> On 2014-12-19 10:55, Sanchayan Maity wrote:
> > Using hw_write_id_reg and hw_read_id_reg to write and read
> > identification registers contents. This can be used to get
> > controller information, change some system configurations
> > and so on.
> 
> Checkpatch is complaining about DOS line endings and some trailing
> whitespace. This applies to all three patches.
> 
> > 
> > Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
> > ---
> >  drivers/usb/chipidea/ci.h |   53 +++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 53 insertions(+)
> > 
> > diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
> > index ea40626..94db636 100644
> > --- a/drivers/usb/chipidea/ci.h
> > +++ b/drivers/usb/chipidea/ci.h
> > @@ -29,6 +29,15 @@
> >  /******************************************************************************
> >   * REGISTERS
> >   *****************************************************************************/
> > +/* Identification Registers */
> > +#define ID_ID				0x0
> > +#define ID_HWGENERAL			0x4
> > +#define ID_HWHOST			0x8
> > +#define ID_HWDEVICE			0xc
> > +#define ID_HWTXBUF			0x10
> > +#define ID_HWRXBUF			0x14
> > +#define ID_SBUSCFG			0x90
> > +
> >  /* register indices */
> >  enum ci_hw_regs {
> >  	CAP_CAPLENGTH,
> > @@ -97,6 +106,18 @@ enum ci_role {
> >  	CI_ROLE_END,
> >  };
> >  
> > +enum CI_REVISION {
> 
> Usually the enum names are small caps, only the labels are capitalized.
> I would suggest use ci_revision here (similar to the enum above).

I will change it.

> 
> > +	CI_REVISION_1X = 10,	/* Revision 1.x */
> > +	CI_REVISION_20 = 20, /* Revision 2.0 */
> > +	CI_REVISION_21, /* Revision 2.1 */
> > +	CI_REVISION_22, /* Revision 2.2 */
> > +	CI_REVISION_23, /* Revision 2.3 */
> > +	CI_REVISION_24, /* Revision 2.4 */
> > +	CI_REVISION_25, /* Revision 2.5 */
> > +	CI_REVISION_25_PLUS, /* Revision above than 2.5 */
> > +	CI_REVISION_UNKNOWN = 99, /* Unknown Revision */
> > +};
> > +
> >  /**
> >   * struct ci_role_driver - host/gadget role driver
> >   * @start: start this role
> > @@ -168,6 +189,7 @@ struct hw_bank {
> >   * @b_sess_valid_event: indicates there is a vbus event, and handled
> >   * at ci_otg_work
> >   * @imx28_write_fix: Freescale imx28 needs swp instruction for writing
> > + * @rev: The revision number for controller
> >   */
> >  struct ci_hdrc {
> >  	struct device			*dev;
> > @@ -207,6 +229,7 @@ struct ci_hdrc {
> >  	bool				id_event;
> >  	bool				b_sess_valid_event;
> >  	bool				imx28_write_fix;
> > +	enum CI_REVISION		rev;
> >  };
> >  
> >  static inline struct ci_role_driver *ci_role(struct ci_hdrc *ci)
> > @@ -244,6 +267,36 @@ static inline void ci_role_stop(struct ci_hdrc *ci)
> >  }
> >  
> >  /**
> > + * hw_read_id_reg: reads from a identification register
> > + * @ci: the controller
> > + * @offset: offset from the beginning of identification registers region
> > + * @mask: bitfield mask
> > + *
> > + * This function returns register contents
> > + */
> > +static inline u32 hw_read_id_reg(struct ci_hdrc *ci, u32 offset, u32 mask)
> > +{
> > +	return ioread32(ci->hw_bank.abs + offset) & mask;
> > +}
> > +
> > +/**
> > + * hw_write_id_reg: writes to a identification register
> > + * @ci: the controller
> > + * @offset: offset from the beginning of identification registers region
> > + * @mask: bitfield mask
> > + * @data: new value
> > + */
> > +static inline void hw_write_id_reg(struct ci_hdrc *ci, u32 offset,
> > +			    u32 mask, u32 data)
> > +{
> > +	if (~mask)
> > +		data = (ioread32(ci->hw_bank.abs + offset) & ~mask)
> > +			| (data & mask);
> > +
> > +	iowrite32(data, ci->hw_bank.abs + offset);
> > +}
> 
> This function isn't used anywhere, does it make sense to write an ID
> register? The ones I see in Vybrid's RM are read-only anyway..
> 

This API is used to write identification registers (offset, 0x00 - 0x90),
not only the ID register, the SBUSCFG (offset 0x90) attribute is read/write.

-- 

Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/