On 3/30/17 3:50 PM, Benjamin Herrenschmidt wrote:
On Thu, 2017-03-30 at 13:15 -0500, Christopher Bostic wrote:
+static void serial_in(struct fsi_master_gpio *master, struct fsi_gpio_msg *msg,
+ uint8_t num_bits)
+{
+ uint8_t bit, in_bit;
+
+ set_sda_input(master);
+
+ for (bit = 0; bit < num_bits; bit++) {
+ clock_toggle(master, 1);
+ in_bit = sda_in(master);
+ msg->msg <<= 1;
+ msg->msg |= ~in_bit & 0x1; /* Data is negative active */
+ }
+ msg->bits += num_bi ts;
+}
+
+static void serial_out(struct fsi_master_gpio *master,
+ const struct fsi_gpio_msg *cmd)
+{
+ uint8_t bit;
+ uint64_t msg = ~cmd->msg; /* Data is negative active */
+ uint64_t sda_mask = 0x1ULL << (cmd->bits - 1);
+ uint64_t last_bit = ~0;
+ int next_bit;
+
+ if (!cmd->bits) {
+ dev_warn(master->dev, "trying to output 0 bits\n");
+ return;
+ }
+ set_sda_output(master, 0);
+
+ /* Send the start bit */
+ sda_out(master, 0);
+ clock_toggle(master, 1);
+
+ /* Send the message */
+ for (bit = 0; bit < cmd->bits; bit++) {
+ next_bit = (msg & sda_mask) >> (cmd->bits - 1);
+ if (last_bit ^ next_bit) {
+ sda_out(master, next_bit);
+ last_bit = next_bit;
+ }
+ clock_toggle(master, 1);
+ msg <<= 1;
+ }
+}
As I mentioned privately, I don't think this is right, unless your
clock signal is inverted or my protocol spec is wrong...
Your clock toggle is written so you call it right after the rising
edge. It does delay, 0, delay, 1.
But according to the FSI timing diagram I have, you need to establish
the data around the falling edge, it gets sampled by the slave on the
rising edge. So as it is, your code risks violating the slave hold
time.
On input, you need to sample on the falling edge, right before it. You
are sampling after the rising edge, so you aren't leaving enough time
for the slave to establish the data.
You could probably just flip clock_toggle() around. Make it: 0, delay,
1, delay.
That way you can do for sends: sda_out + toggle, and for receive
toggle + sda_in. That will make you establish your output data and
sample right before the falling edge, which should be ok provided the
diagram I have is right.
Hi Ben,
Agreed that there is room for improvement. I intend to look further
into your suggestions from here and our private conversation on the
matter and make changes as appropriate. I have an open issue to track
this. As it exists in this patch reads/writes from master to slave
fundamentally work. Given the pervasiveness and time to fully evaluate
and test any protocol updates I intend address this in the near future
with a separate follow on patch.
Thanks,
Chris
Cheers,
Ben.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
