From: David Brownell <david-b@pacbell•net>
To: Peter Korsgaard <jacmet@sunsite•dk>
Cc: linuxppc-dev@ozlabs•org, dbrownell@users•sourceforge.net,
linux-usb@vger•kernel.org
Subject: Re: [patch v6 1/4] USB: add Cypress c67x00 low level interface code
Date: Fri, 1 Feb 2008 13:54:11 -0800 [thread overview]
Message-ID: <200802011354.11819.david-b@pacbell.net> (raw)
In-Reply-To: <20080129123119.273749000@sunsite.dk>
On Tuesday 29 January 2008, Peter Korsgaard wrote:
> This patch adds the low level support code for the Cypress c67x00 family of
> OTG controllers. The low level code is responsible for register access and
> implements the software protocol for communicating with the 16bit
> microcontroller inside the c67x00 device.
>
> Communication is done over the HPI interface (16bit SRAM-like parallel bus).
>
> Signed-off-by: Peter Korsgaard <jacmet@sunsite•dk>
If you fix the issues I note below:
Acked-by: David Brownell <dbrownell@users•sourceforge.net>
> +/**
> + * struct c67x00_sie - Common data associated with a SIE
> + * @lock: lock to protect this struct
"and the associated chip registers"
> + * @private_data: subdriver dependent data
> + * @irq: subdriver dependent irq handler, set NULL when not used
> + * @dev: link to common driver structure
> + * @sie_num: SIE number on chip, starting from 0
> + * @mode: SIE mode (host/peripheral/otg/not used)
> + */
> +struct c67x00_sie {
> + /* Entries to be used by the subdrivers */
> + spinlock_t lock; /* protect this structure */
> + void *private_data;
> + void (*irq) (struct c67x00_sie *sie, u16 int_status, u16 msg);
> +
> + /* Read only: */
> + struct c67x00_device *dev;
> + int sie_num;
> + int mode;
> +};
In the C file:
> +static inline u16 hpi_read_word(struct c67x00_device *dev, u16 reg)
> +{
> + u16 value;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&dev->hpi.lock, flags);
> + value = hpi_read_word_nolock(dev, reg);
> + spin_unlock_irqrestore(&dev->hpi.lock, flags);
> +
> + return value;
> +}
> +
> +static inline void hpi_write_word_nolock(struct c67x00_device *dev, u16 reg,
> + u16 value)
> +{
> + hpi_write_reg(dev, HPI_ADDR, reg);
> + hpi_write_reg(dev, HPI_DATA, value);
> +}
> +
> +static inline void hpi_write_word(struct c67x00_device *dev, u16 reg, u16 value)
> +{
> + unsigned long flags;
> +
> + spin_lock_irqsave(&dev->hpi.lock, flags);
> + hpi_write_word_nolock(dev, reg, value);
> + spin_unlock_irqrestore(&dev->hpi.lock, flags);
> +}
> +
> +/*
> + * Only data is little endian, addr has cpu endianess
> + */
> +static inline void hpi_write_words_le16(struct c67x00_device *dev, u16 addr,
> + u16 *data, u16 count)
> +{
> + unsigned long flags;
> + int i;
> +
> + spin_lock_irqsave(&dev->hpi.lock, flags);
> +
> + hpi_write_reg(dev, HPI_ADDR, addr);
> + for (i = 0; i < count; i++)
> + hpi_write_reg(dev, HPI_DATA, cpu_to_le16(*data++));
> +
> + spin_unlock_irqrestore(&dev->hpi.lock, flags);
> +}
> +
> +/*
> + * Only data is little endian, addr has cpu endianess
> + */
> +static inline void hpi_read_words_le16(struct c67x00_device *dev, u16 addr,
> + u16 *data, u16 count)
> +{
> + unsigned long flags;
> + int i;
> + spin_lock_irqsave(&dev->hpi.lock, flags);
> + hpi_write_reg(dev, HPI_ADDR, addr);
> + for (i = 0; i < count; i++)
> + *data++ = le16_to_cpu(hpi_read_reg(dev, HPI_DATA));
> +
> + spin_unlock_irqrestore(&dev->hpi.lock, flags);
> +}
> +
> +static inline void hpi_set_bits(struct c67x00_device *dev, u16 reg, u16 mask)
> +{
> + u16 value;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&dev->hpi.lock, flags);
> + value = hpi_read_word_nolock(dev, reg);
> + hpi_write_word_nolock(dev, reg, value | mask);
> + spin_unlock_irqrestore(&dev->hpi.lock, flags);
> +}
> +
> +static inline void hpi_clear_bits(struct c67x00_device *dev, u16 reg, u16 mask)
> +{
> + u16 value;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&dev->hpi.lock, flags);
> + value = hpi_read_word_nolock(dev, reg);
> + hpi_write_word_nolock(dev, reg, value & ~mask);
> + spin_unlock_irqrestore(&dev->hpi.lock, flags);
> +}
> +
> +static inline u16 hpi_recv_mbox(struct c67x00_device *dev)
> +{
> + u16 value;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&dev->hpi.lock, flags);
> + value = hpi_read_reg(dev, HPI_MAILBOX);
> + spin_unlock_irqrestore(&dev->hpi.lock, flags);
> +
> + return value;
> +}
> +
> +static inline u16 hpi_send_mbox(struct c67x00_device *dev, u16 value)
> +{
> + unsigned long flags;
> +
> + spin_lock_irqsave(&dev->hpi.lock, flags);
> + hpi_write_reg(dev, HPI_MAILBOX, value);
> + spin_unlock_irqrestore(&dev->hpi.lock, flags);
> +
> + return value;
> +}
Strike the "inline" from all the above, and let the compiler decide
if the space savings are worthwhile. (I'd guess: mostly not.)
Given icache, time savings are likely negligible.
next prev parent reply other threads:[~2008-02-01 21:54 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-01-29 12:24 [patch v6 0/4] Cypress c67x00 (EZ-Host/EZ-OTG) support Peter Korsgaard
2008-01-29 12:24 ` [patch v6 1/4] USB: add Cypress c67x00 low level interface code Peter Korsgaard
2008-02-01 21:54 ` David Brownell [this message]
2008-01-29 12:24 ` [patch v6 2/4] USB: add Cypress c67x00 OTG controller core driver Peter Korsgaard
2008-02-01 21:58 ` David Brownell
2008-01-29 12:24 ` [patch v6 3/4] USB: add Cypress c67x00 OTG controller HCD driver Peter Korsgaard
2008-01-29 15:27 ` Alan Stern
2008-01-30 9:35 ` Peter Korsgaard
2008-01-30 15:19 ` Alan Stern
2008-01-29 12:24 ` [patch v6 4/4] USB: add Cypress c67x00 OTG controller gadget driver Peter Korsgaard
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=200802011354.11819.david-b@pacbell.net \
--to=david-b@pacbell$(echo .)net \
--cc=dbrownell@users$(echo .)sourceforge.net \
--cc=jacmet@sunsite$(echo .)dk \
--cc=linux-usb@vger$(echo .)kernel.org \
--cc=linuxppc-dev@ozlabs$(echo .)org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox