On 10/25/2015 02:02 PM, Robert Jarzmik wrote:
Jarkko Nikula <jarkko.nikula@xxxxxxxxxxxxxxx> writes:
LPSS SPI devices in a successor of Intel Sunrisepoint can have up to 4 chip
is ?
Intel Low Power Subsystem SPI host controller(s) after Sunrisepoint can
have multiple chip selects. I'll clarify this since patch 7/7 outdates
the comment by adding support for two variants.
@@ -64,6 +65,10 @@ MODULE_ALIAS("platform:pxa2xx-spi");
#define GENERAL_REG_RXTO_HOLDOFF_DISABLE BIT(24)
#define SPI_CS_CONTROL_SW_MODE BIT(0)
#define SPI_CS_CONTROL_CS_HIGH BIT(1)
+#define SPI_CS_CONTROL_CS_SEL_SHIFT 8
+#define SPI_CS_CONTROL_CS_SEL_MASK (3 << SPI_CS_CONTROL_CS_SEL_SHIFT)
+#define SPI_CAPS_CS_EN_SHIFT 9
+#define SPI_CAPS_CS_EN_MASK (0xf << SPI_CAPS_CS_EN_SHIFT)
This is intel specific, not pxa oriented, right ?
If so, I'd like to have it namespaced, with LPSS or whatever you see fit.
Good point. I'll check also couple other existing defines as they are
LPSS specific.
@@ -103,6 +111,7 @@ static const struct lpss_config lpss_platforms[] = {
.reg_general = -1,
.reg_ssp = 0x20,
.reg_cs_ctrl = 0x24,
+ .reg_capabilities = 0xfc,
Immediate value ? I suppose a defines usage was not possible here ?
I removed register defines for these three registers earlier when code
started accessing registers using this configuration data. There are
differences in these registers between platforms and it didn't look much
clearer to have separate defines for them.
@@ -271,15 +280,34 @@ static void lpss_ssp_setup(struct driver_data *drv_data)
static void lpss_ssp_cs_control(struct driver_data *drv_data, bool enable)
{
const struct lpss_config *config;
- u32 value;
+ u32 value, cs;
config = lpss_get_config(drv_data);
value = __lpss_ssp_read_priv(drv_data, config->reg_cs_ctrl);
- if (enable)
+ if (enable) {
+ cs = drv_data->cur_msg->spi->chip_select;
+ cs <<= SPI_CS_CONTROL_CS_SEL_SHIFT;
+ if (cs != (value & SPI_CS_CONTROL_CS_SEL_MASK)) {
+ /*
+ * When switching another chip select output active
+ * the output must be selected first and wait 2 ssp_clk
+ * cycles before changing state to active. Otherwise
+ * a short glitch will occur on the previous chip
+ * select since output select is latched but state
+ * control is not.
+ */
+ value &= ~SPI_CS_CONTROL_CS_SEL_MASK;
+ value |= cs;
+ __lpss_ssp_write_priv(drv_data,
+ config->reg_cs_ctrl, value);
+ ndelay(1000000000 /
+ (drv_data->master->max_speed_hz / 2));
That ndelay() looks weird, why delay and not sleep ?
And wouldn't that be targeted at another patch ? If I got it right, this patch
deals with capabilites, not chip select timings, doesn't it ?
Typically we are seeing input clock of SSP being around a few or tens of
megahertz so sleep instead of <1 us delay would slow down the transfers
needlessly.
That said, we may have a need to change this to use delay or sleep
conditionally later. Andy has been looking at changing input clock
according to transfer speed which means that needed delay can be up to
milliseconds if we use really low transfer speed.
Actually this patch deals with output control and number of chip select
detection. Maybe it calls for two patches. Output control prepares for
multiple chip selects and detection adds actual support for them.
--
Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
