Re: [2/2] Driver for the Maxim DS1WM, a 1-wire bus master ASIC core.

[pHilipp Zabel]

On 4/26/07, Matt Reimer <mreimer@xxxxxxxx> wrote:
> On Apr 25, 2007, at 5:46 PM, Andrew Morton wrote:
>
> > On Tue, 24 Apr 2007 14:02:03 +0400 Evgeniy Polyakov
> > <johnpol@xxxxxxxxxxx> wrote:
> >
> >> +#define DS1WM_CMD_1W_RESET  1 << 0  /* force reset on 1-wire bus */
> >> +#define DS1WM_CMD_SRA           1 << 1      /* enable Search ROM accelerator
> >> mode */
> >> +#define DS1WM_CMD_DQ_OUTPUT 1 << 2  /* write only - forces bus low */
> >> +#define DS1WM_CMD_DQ_INPUT  1 << 3  /* read only - reflects state
> >> of bus */
> >> +
> >> +#define DS1WM_INT_PD            1 << 0      /* presence detect */
> >> +#define DS1WM_INT_PDR           1 << 1      /* presence detect result */
> >> +#define DS1WM_INT_TBE           1 << 2      /* tx buffer empty */
> >> +#define DS1WM_INT_TSRE          1 << 3      /* tx shift register empty */
> >> +#define DS1WM_INT_RBF           1 << 4      /* rx buffer full */
> >> +#define DS1WM_INT_RSRF          1 << 5      /* rx shift register full */
> >> +
> >> +#define DS1WM_INTEN_EPD         1 << 0      /* enable presence detect int */
> >> +#define DS1WM_INTEN_IAS         1 << 1      /* INTR active state */
> >> +#define DS1WM_INTEN_ETBE    1 << 2  /* enable tx buffer empty int */
> >> +#define DS1WM_INTEN_ETMT    1 << 3  /* enable tx shift register
> >> empty int */
> >> +#define DS1WM_INTEN_ERBF    1 << 4  /* enable rx buffer full int */
> >> +#define DS1WM_INTEN_ERSRF   1 << 5  /* enable rx shift register
> >> full int */
> >> +#define DS1WM_INTEN_DQO         1 << 6      /* enable direct bus driving ops
> >> +                                       (undocumented), Szabolcs Gyurko */
> >
> > These macros are very dangerous - please parenthesise them all.
>
> (and several other helpful suggestions...)
>
> Thanks for your review Andrew. I made all the code changes you
> suggested in the attached patch.
>
> Matt

diff --git a/drivers/w1/masters/Makefile b/drivers/w1/masters/Makefile
index 4cee256..a9e45fb 100644
--- a/drivers/w1/masters/Makefile
+++ b/drivers/w1/masters/Makefile
@@ -5,4 +5,4 @@
obj-$(CONFIG_W1_MASTER_MATROX)		+= matrox_w1.o
obj-$(CONFIG_W1_MASTER_DS2490)		+= ds2490.o
obj-$(CONFIG_W1_MASTER_DS2482)		+= ds2482.o
-
+obj-$(CONFIG_W1_DS1WM)                  += ds1wm.o

Wouldn't it be better to call the config option CONFIG_W1_MASTER_DS1WM
for consistency?

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