| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <c38f5f71f018cbbb71f8e544735c59a3@agner.ch>
Date: Wed, 24 Dec 2014 17:41:29 +0100
From: Stefan Agner <stefan@...er.ch>
To: Sanchayan Maity <maitysanchayan@...il.com>
Cc: Peter.Chen@...escale.com, gregkh@...uxfoundation.org,
linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/3] usb: chipidea: Add identification registers access
APIs
On 2014-12-24 17:15, Sanchayan Maity wrote:
> On 12/24/2014 09:30 PM, 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.
>
> Hmm... that's surprising. I did run checkpatch at my end before sending.
> I do not have the original patchset on my laptop which I have right now
> but downloading diffs back from lkml and running checkpatch on them back,
> only gives me signed off errors as it should.
>
You are right, it's on my side. My webmailer (Roundcube) seem to
introduce those line endings when using "Download (.eml)". Using "Show
Source", and then save the e-mail makes checkpatch happy... Sorry about
that.
--
Stefan
> I will resend if somehow errors crept in with my earlier patches.
>
> -Sanchayan
>
>>
>>>
>>> Signed-off-by: Sanchayan Maity <maitysanchayan@...il.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).
>>
>>> + 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..
>>
>> --
>> Stefan
>>
>>> +
>>> +/**
>>> * hw_read: reads from a hw register
>>> * @ci: the controller
>>> * @reg: register index
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists