All of lore.kernel.org
 help / color / mirror / Atom feed
From: 敬锐 <micky_ching@realsil.com.cn>
To: Lee Jones <lee.jones@linaro.org>
Cc: "ulf.hansson@linaro.org" <ulf.hansson@linaro.org>,
	"sameo@linux.intel.com" <sameo@linux.intel.com>,
	"gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
	"linux-mmc@vger.kernel.org" <linux-mmc@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"chris@printf.net" <chris@printf.net>,
	王炜 <wei_wang@realsil.com.cn>,
	"rogerable@realtek.com" <rogerable@realtek.com>,
	"devel@linuxdriverproject.org" <devel@linuxdriverproject.org>,
	"dan.carpenter@oracle.com" <dan.carpenter@oracle.com>
Subject: Re: [PATCH v4 1/6] mfd: rtsx: add func to split u32 into register
Date: Mon, 8 Dec 2014 09:49:29 +0000	[thread overview]
Message-ID: <54857429.5060108@realsil.com.cn> (raw)
In-Reply-To: <20141208084941.GC3951@x1>


On 12/08/2014 04:49 PM, Lee Jones wrote:
>> diff --git a/include/linux/mfd/rtsx_pci.h b/include/linux/mfd/rtsx_pci.h
>> >index 74346d5..9234449 100644
>> >--- a/include/linux/mfd/rtsx_pci.h
>> >+++ b/include/linux/mfd/rtsx_pci.h
>> >@@ -558,6 +558,7 @@
>> >  #define SD_SAMPLE_POINT_CTL		0xFDA7
>> >  #define SD_PUSH_POINT_CTL		0xFDA8
>> >  #define SD_CMD0				0xFDA9
>> >+#define   SD_CMD_START			0x40
> This is a different format at the others in the group on two counts;
> firstly you have 3 whitespace characters after the #define and
> secondly all of the other hex digits in the set are 4 a breast.
> Please add the leading zeros to conform to this format.
>
Hi lee,

This format is more readable, add 2 space means this value is
in groups under register SD_CMD0, and each register value is u8 type,
so use 2 hex digits is right.

The original file make register address and its related value separated,
and group register together.

But I like to write register address and value together,
add 2 space for value to indicate this value is related to above register.

If we add new address/value, I recommend write register define format as 
below:

#define REGISTER        addr
#define   REG_VAL1      val1
#define   REG_VAL2      val2

regards.
micky.

WARNING: multiple messages have this Message-ID (diff)
From: 敬锐 <micky_ching@realsil.com.cn>
To: Lee Jones <lee.jones@linaro.org>
Cc: "sameo@linux.intel.com" <sameo@linux.intel.com>,
	"chris@printf.net" <chris@printf.net>,
	"ulf.hansson@linaro.org" <ulf.hansson@linaro.org>,
	"devel@linuxdriverproject.org" <devel@linuxdriverproject.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-mmc@vger.kernel.org" <linux-mmc@vger.kernel.org>,
	"gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
	"dan.carpenter@oracle.com" <dan.carpenter@oracle.com>,
	"rogerable@realtek.com" <rogerable@realtek.com>,
	王炜 <wei_wang@realsil.com.cn>
Subject: Re: [PATCH v4 1/6] mfd: rtsx: add func to split u32 into register
Date: Mon, 8 Dec 2014 09:49:29 +0000	[thread overview]
Message-ID: <54857429.5060108@realsil.com.cn> (raw)
In-Reply-To: <20141208084941.GC3951@x1>

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1418 bytes --]


On 12/08/2014 04:49 PM, Lee Jones wrote:
>> diff --git a/include/linux/mfd/rtsx_pci.h b/include/linux/mfd/rtsx_pci.h
>> >index 74346d5..9234449 100644
>> >--- a/include/linux/mfd/rtsx_pci.h
>> >+++ b/include/linux/mfd/rtsx_pci.h
>> >@@ -558,6 +558,7 @@
>> >  #define SD_SAMPLE_POINT_CTL		0xFDA7
>> >  #define SD_PUSH_POINT_CTL		0xFDA8
>> >  #define SD_CMD0				0xFDA9
>> >+#define   SD_CMD_START			0x40
> This is a different format at the others in the group on two counts;
> firstly you have 3 whitespace characters after the #define and
> secondly all of the other hex digits in the set are 4 a breast.
> Please add the leading zeros to conform to this format.
>
Hi lee,

This format is more readable, add 2 space means this value is
in groups under register SD_CMD0, and each register value is u8 type,
so use 2 hex digits is right.

The original file make register address and its related value separated,
and group register together.

But I like to write register address and value together,
add 2 space for value to indicate this value is related to above register.

If we add new address/value, I recommend write register define format as 
below:

#define REGISTER        addr
#define   REG_VAL1      val1
#define   REG_VAL2      val2

regards.
micky.ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

  reply	other threads:[~2014-12-08  9:49 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-05  5:54 [PATCH v4 0/6] mmc: rtsx: add support for sdio card micky_ching
2014-12-05  5:54 ` micky_ching
2014-12-05  5:54 ` [PATCH v4 1/6] mfd: rtsx: add func to split u32 into register micky_ching
2014-12-05  5:54   ` micky_ching
2014-12-05  9:27   ` 敬锐
2014-12-05  9:27     ` 敬锐
2014-12-08  8:49   ` Lee Jones
2014-12-08  9:49     ` 敬锐 [this message]
2014-12-08  9:49       ` 敬锐
2014-12-08  9:57       ` Lee Jones
2014-12-08  9:59         ` 敬锐
2014-12-08  9:59           ` 敬锐
2014-12-08  9:57   ` Lee Jones
2014-12-08  9:57     ` Lee Jones
2014-12-08 10:21     ` Ulf Hansson
2014-12-08 10:21       ` Ulf Hansson
2014-12-08 13:01       ` Lee Jones
2014-12-08 13:01         ` Lee Jones
2014-12-05  5:54 ` [PATCH v4 2/6] mmc: rtsx: add dump_reg_range to simplify dump register micky_ching
2014-12-05  5:54   ` micky_ching
2014-12-05  5:54 ` [PATCH v4 3/6] mmc: rtsx: init cookie at probe/card_event micky_ching
2014-12-05  5:54   ` micky_ching
2014-12-05  5:54 ` [PATCH v4 4/6] mmc: rtsx: add helper function to simplify code micky_ching
2014-12-05  5:54   ` micky_ching
2014-12-05  5:54 ` [PATCH v4 5/6] mmc: rtsx: add support for sdio card micky_ching
2014-12-05  5:54   ` micky_ching
2014-12-05  5:54 ` [PATCH v4 6/6] mmc: rtsx: swap function position to avoid pre declaration micky_ching
2014-12-05  5:54   ` micky_ching
2014-12-19 10:36 ` [PATCH v4 0/6] mmc: rtsx: add support for sdio card Ulf Hansson
2014-12-19 10:36   ` Ulf Hansson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=54857429.5060108@realsil.com.cn \
    --to=micky_ching@realsil.com.cn \
    --cc=chris@printf.net \
    --cc=dan.carpenter@oracle.com \
    --cc=devel@linuxdriverproject.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=lee.jones@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mmc@vger.kernel.org \
    --cc=rogerable@realtek.com \
    --cc=sameo@linux.intel.com \
    --cc=ulf.hansson@linaro.org \
    --cc=wei_wang@realsil.com.cn \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.