Re: [PATCH, RFC] Freescale STMP: i2c driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Jun 16, 2009 at 08:25:39PM +0400, dmitry pervushin wrote:
> Hello Ben,
> 
> I am sending the updated patch in next message, and this message is just
> to answer your questions/concerns.\
> 
> [...]
> > > +#include <mach/regs-apbx.h>
> > > +
> > > +static const u32 I2C_READ = 1,
> > > +		 I2C_WRITE = 0;
> > 
> > do you really want to be defining things with a prefix of I2C, thatB
> > might end up clashing with the i2c core?
> Fixed, changed to I2C_SMBUS_READ/WRITE as Jean Delvare proposed.
> > 
> > > +static const struct stmp3xxx_dma_command cmd_i2c_select = {
> > > +	.cmd =	BF(1, APBX_CHn_CMD_XFER_COUNT)	|
> > > +		BF(1, APBX_CHn_CMD_CMDWORDS)	|
> > > +		BM_APBX_CHn_CMD_WAIT4ENDCMD	|
> > > +		BM_APBX_CHn_CMD_CHAIN		|
> > > +		BF(BV_APBX_CHn_CMD_COMMAND__DMA_READ, APBX_CHn_CMD_COMMAND),
> > 
> > what is BF() ?
> it is the macro from arch/arm/plat-stmp3xxx/include/mach/platform.h,
> abbreviated "bitfield" :) For example, BF(1, APBX_CHn_CMD_CMDWORDS) is
> expanded like 
> ((1 << BP_APBX_CHn_CMD_CMDWORDS) & BM_APBX_CHn_CMD_CMDWORDS).
> BP_xxx stuff means bitfield position, BM_xxx - bitfield mask. 

yuck. I don't like people hiding thinks in macros for what is very
little gain, especially when an extant constant is being used!

-- 
Ben (ben@xxxxxxxxx, http://www.fluff.org/)

  'a smiley only costs 4 bytes'
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux GPIO]     [Linux SPI]     [Linux Hardward Monitoring]     [LM Sensors]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux