Re: [U-Boot] [PATCH v1 1/3] arm: socfpga: stratix10: Add Stratix10 FPGA Reconfiguration Driver

[Ang, Chee Hong]

On Fri, 2018-04-27 at 09:08 +0200, Marek Vasut wrote:
> On 04/27/2018 07:51 AM, Ang, Chee Hong wrote:
> [...]
> 
> > 
> > > 
> > > > 
> > > > > 
> > > > > > 
> > > > > > +   /* Check for any error */
> > > > > > +   ret =
> > > > > > reconfig_status_resp[RECONFIG_STATUS_STATE];
> > > > > > +   if (ret && ret !=
> > > > > > MBOX_CFGSTAT_STATE_CONFIG)
> > > > > > +   return ret;
> > > > > > +
> > > > > > +   /* Make sure nStatus is not 0 */
> > > > > > +   ret =
> > > > > > reconfig_status_resp[RECONFIG_STATUS_PIN_STATUS];
> > > > > > +   if (!(ret & RCF_PIN_STATUS_NSTATUS))
> > > > > > +   return
> > > > > > MBOX_CFGSTAT_STATE_ERROR_HARDWARE;
> > > > > wait_for_bit_le32() or somesuch ?
> > > > No, we don't read the bit status directly from register. We
> > > > only
> > > > need
> > > > to test the bit of the pin status stored in memory.
> > > Who's populating the memory ?
> > ret = mbox_send_cmd(MBOX_ID_UBOOT, MBOX_RECONFIG_STATUS,
> > MBOX_CMD_DIRECT, 0, NULL, 0, _status_resp_len,
> > reconfig_status_resp);
> > 
> > We send mailbox command to the device and the device will respond
> > by
> > filling the 'reconfig_status_resp' with the device status.
> Ah, I see. Would it make sense to implement a generic "poll for bit"
> for
> this mailbox communication ? Also, we have drivers/mailbox/ , would
> it
> make sense to move the mailbox stuff there ?
There is a separate patch for mailbox communication stuffs in the
process of upstreaming by my colleague Tan, Ley Foon. But currently, I
don't think the mailbox code is placed under drivers/mailbox. I can
refactor this code into a generic function like 'get fpga device
status' via mailbox and place it under drivers/mailbox. Will work with
her to address this.
> 
> [...]
> 
> > 
> > > 
> > > > 
> > > > > 
> > > > > > 
> > > > > > +   if (*resp_count < buf_size) {
> > > > > > +   u32 rcv_len_max = buf_size - *resp_count;
> > > > > > +
> > > > > > +   if (rcv_len_max > MBOX_RESP_BUFFER_SIZE)
> > > > > > +   rcv_len_max =
> > > > > > MBOX_RESP_BUFFER_SIZE;
> > > > > > +   resp_len = mbox_rcv_resp(buf,
> > > > > > rcv_len_max);
> > > > > > +
> > > > > > +   for (i = 0; i < resp_len; i++) {
> > > > > > +   resp_buf[(*w_index)++] = buf[i];
> > > > > Is this like a memcpy() ?
> > > > No. This is a circular buffer, index to the memory location may
> > > > wrap
> > > > around.
> > > Two memcpys then ?
> > Will refactor this part in v2.
> Does it even have to be a circular buffer in the first place ?
Yes, these mailbox responses are stored in buffer and will be retrieved
in sequence at later stage.
> 
> [...]
> 
> > 
> > > 
> > > > 
> > > > > 
> > > > > > 
> > > > > > +   /* Make sure we don't send MBOX_RECONFIG_STATUS
> > > > > > too
> > > > > > fast
> > > > > > */
> > > > > > +   udelay(RECONFIG_STATUS_INTERVAL_DELAY_US);
> > > > > Hum, this is iffy, is that a hardware bug ?
> > > > Yes. The firmware running in that device is not able to
> > > > response
> > > > quickly.
> > > And you cannot poll it in some way ?
> > I know this is ugly and looks buggy. But there are no mechanism to
> > poll
> > whether the device is ready to accept any mailbox command or not.
> > So
> > for now, slowing down the mailbox exchange is the only way to go.
> If it works reliably, so be it.
Yes. It works reliably.
> 
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH v1 1/3] arm: socfpga: stratix10: Add Stratix10 FPGA Reconfiguration Driver

[Ang, Chee Hong]

On Thu, 2018-04-26 at 14:37 +0200, Marek Vasut wrote:
> On 04/26/2018 08:12 AM, Ang, Chee Hong wrote:
> > 
> > On Fri, 2018-04-20 at 05:41 +0200, Marek Vasut wrote:
> > > 
> > > On 04/20/2018 05:26 AM, chee.hong@intel.com wrote:
> > > > 
> > > > 
> > > > From: Chee Hong Ang 
> > > > 
> > > > Enable FPGA reconfiguration support on Stratix10 SoC.
> > > > 
> > > > Signed-off-by: Chee Hong Ang 
> > > > ---
> > > >  drivers/fpga/Kconfig |  10 ++
> > > >  drivers/fpga/Makefile|   1 +
> > > >  drivers/fpga/stratix10.c | 298
> > > > +++
> > > >  3 files changed, 309 insertions(+)
> > > >  create mode 100644 drivers/fpga/stratix10.c
> > > > 
> > > > diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
> > > > index d36c4c5..255683d 100644
> > > > --- a/drivers/fpga/Kconfig
> > > > +++ b/drivers/fpga/Kconfig
> > > > @@ -31,6 +31,16 @@ config FPGA_CYCLON2
> > > >       Enable FPGA driver for loading bitstream in BIT and
> > > > BIN
> > > > format
> > > >       on Altera Cyclone II device.
> > > >  
> > > > +config FPGA_STRATIX10
> > > > +   bool "Enable Altera FPGA driver for Stratix 10"
> > > > +   depends on FPGA_ALTERA && TARGET_SOCFPGA_STRATIX10
> > > > +   help
> > > > +     Say Y here to enable the Altera Stratix 10 FPGA
> > > > specific
> > > > driver
> > > > +
> > > > +     This provides common functionality for Altera
> > > > Stratix 10
> > > > devices.
> > > > +     Enable FPGA driver for writing bitstream into Altera
> > > > Stratix10
> > > > +     device.
> > > > +
> > > >  config FPGA_XILINX
> > > >     bool "Enable Xilinx FPGA drivers"
> > > >     select FPGA
> > > > diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
> > > > index 08c9ff8..3c906ec 100644
> > > > --- a/drivers/fpga/Makefile
> > > > +++ b/drivers/fpga/Makefile
> > > > @@ -19,6 +19,7 @@ obj-$(CONFIG_FPGA_ACEX1K) += ACEX1K.o
> > > >  obj-$(CONFIG_FPGA_CYCLON2) += cyclon2.o
> > > >  obj-$(CONFIG_FPGA_STRATIX_II) += stratixII.o
> > > >  obj-$(CONFIG_FPGA_STRATIX_V) += stratixv.o
> > > > +obj-$(CONFIG_FPGA_STRATIX10) += stratix10.o
> > > >  obj-$(CONFIG_FPGA_SOCFPGA) += socfpga.o
> > > >  obj-$(CONFIG_TARGET_SOCFPGA_GEN5) += socfpga_gen5.o
> > > >  obj-$(CONFIG_TARGET_SOCFPGA_ARRIA10) += socfpga_arria10.o
> > > > diff --git a/drivers/fpga/stratix10.c
> > > > b/drivers/fpga/stratix10.c
> > > > new file mode 100644
> > > > index 000..f0c5ace
> > > > --- /dev/null
> > > > +++ b/drivers/fpga/stratix10.c
> > > > @@ -0,0 +1,298 @@
> > > > +// SPDX-License-Identifier: GPL-2.0
> > > > +/*
> > > > + * Copyright (C) 2016-2018 Intel Corporation 
> > > > + *
> > > > + */
> > > > +
> > > > +#include 
> > > > +#include 
> > > > +#include 
> > > > +
> > > > +#define RECONFIG_STATUS_POLL_RESP_TIMEOUT_MS   60
> > > > 000
> > > > +#define RECONFIG_STATUS_INTERVAL_DELAY_US  1
> > > > 00
> > > > +
> > > > +static const struct mbox_cfgstat_state {
> > > > +   int err_no;
> > > > +   const char  *error_name;
> > > > +} mbox_cfgstat_state[] = {
> > > > +   {MBOX_CFGSTAT_STATE_IDLE, "FPGA in idle mode."},
> > > > +   {MBOX_CFGSTAT_STATE_CONFIG, "FPGA in config mode."},
> > > > +   {MBOX_CFGSTAT_STATE_FAILACK, "Acknowledgement
> > > > failed!"},
> > > > +   {MBOX_CFGSTAT_STATE_ERROR_INVALID, "Invalid
> > > > bitstream!"},
> > > > +   {MBOX_CFGSTAT_STATE_ERROR_CORRUPT, "Corrupted
> > > > bitstream!"},
> > > > +   {MBOX_CFGSTAT_STATE_ERROR_AUTH, "Authentication
> > > > failed!"},
> > > > +   {MBOX_CFGSTAT_STATE_ERROR_CORE_IO, "I/O error!"},
> > > > +   {MBOX_CFGSTAT_STATE_ERROR_HARDWARE, "Hardware
> > > > error!"},
> > > > +   {MBOX_CFGSTAT_STATE_ERROR_FAKE, "Fake error!"},
> > > > +   {MBOX_CFGSTAT_STATE_ERROR_BOOT_INFO, "Error in boot
> > > > info!"},
> > > > +   {MBOX_CFGSTAT_STATE_ERROR_QSPI_ERROR, "Error in
> > > > QSPI!"},
> > > > +   {-ETIMEDOUT, "I/O timeout error"},
> > > > +   {-1, "Unknown error!"}
> > > > +};
> > > > +
> > > > +#define MBOX_CFGSTAT_MAX \
> > > > +     (sizeof(mbox_cfgstat_state) / sizeof(struct
> > > > mbox_cfgstat_state))
> > > > +
> > > > +static const char *mbox_cfgstat_to_str(int err)
> > > > +{
> > > > +   int i;
> > > > +
> > > > +   for (i = 0; i < MBOX_CFGSTAT_MAX - 1; i++) {
> > > > +   if (mbox_cfgstat_state[i].err_no == err)
> > > > +   return
> > > > mbox_cfgstat_state[i].error_name;
> > > > +   }
> > > > +
> > > > +   return mbox_cfgstat_state[MBOX_CFGSTAT_MAX -
> > > > 1].error_name;
> > > > +}
> > > > +
> > > > +static void inc_cmd_id(u8 *id)
> > > > +{
> > > > +   u8 val = *id + 1;
> > > > +
> > > > +   if (val > 15)
> > > > +   val = 1;
> > > What's this function about, elaborate implementation of (i % 15)
> > > + 1
> > > ?
> > This function increment the 4 bits transaction ID 

Re: [U-Boot] [PATCH v1 1/3] arm: socfpga: stratix10: Add Stratix10 FPGA Reconfiguration Driver

[Marek Vasut]

On 04/27/2018 07:51 AM, Ang, Chee Hong wrote:
[...]

> + /* Check for any error */
> + ret =
> reconfig_status_resp[RECONFIG_STATUS_STATE];
> + if (ret && ret != MBOX_CFGSTAT_STATE_CONFIG)
> + return ret;
> +
> + /* Make sure nStatus is not 0 */
> + ret =
> reconfig_status_resp[RECONFIG_STATUS_PIN_STATUS];
> + if (!(ret & RCF_PIN_STATUS_NSTATUS))
> + return
> MBOX_CFGSTAT_STATE_ERROR_HARDWARE;
 wait_for_bit_le32() or somesuch ?
>>> No, we don't read the bit status directly from register. We only
>>> need
>>> to test the bit of the pin status stored in memory.
>> Who's populating the memory ?
> ret = mbox_send_cmd(MBOX_ID_UBOOT, MBOX_RECONFIG_STATUS,
> MBOX_CMD_DIRECT, 0, NULL, 0, _status_resp_len,
> reconfig_status_resp);
> 
> We send mailbox command to the device and the device will respond by
> filling the 'reconfig_status_resp' with the device status.

Ah, I see. Would it make sense to implement a generic "poll for bit" for
this mailbox communication ? Also, we have drivers/mailbox/ , would it
make sense to move the mailbox stuff there ?

[...]

> + if (*resp_count < buf_size) {
> + u32 rcv_len_max = buf_size - *resp_count;
> +
> + if (rcv_len_max > MBOX_RESP_BUFFER_SIZE)
> + rcv_len_max = MBOX_RESP_BUFFER_SIZE;
> + resp_len = mbox_rcv_resp(buf, rcv_len_max);
> +
> + for (i = 0; i < resp_len; i++) {
> + resp_buf[(*w_index)++] = buf[i];
 Is this like a memcpy() ?
>>> No. This is a circular buffer, index to the memory location may
>>> wrap
>>> around.
>> Two memcpys then ?
> Will refactor this part in v2.

Does it even have to be a circular buffer in the first place ?

[...]

> + /* Make sure we don't send MBOX_RECONFIG_STATUS too
> fast
> */
> + udelay(RECONFIG_STATUS_INTERVAL_DELAY_US);
 Hum, this is iffy, is that a hardware bug ?
>>> Yes. The firmware running in that device is not able to response
>>> quickly.
>> And you cannot poll it in some way ?
> I know this is ugly and looks buggy. But there are no mechanism to poll
> whether the device is ready to accept any mailbox command or not. So
> for now, slowing down the mailbox exchange is the only way to go.

If it works reliably, so be it.

-- 
Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH v1 1/3] arm: socfpga: stratix10: Add Stratix10 FPGA Reconfiguration Driver

[Marek Vasut]

On 04/26/2018 08:12 AM, Ang, Chee Hong wrote:
> On Fri, 2018-04-20 at 05:41 +0200, Marek Vasut wrote:
>> On 04/20/2018 05:26 AM, chee.hong@intel.com wrote:
>>>
>>> From: Chee Hong Ang 
>>>
>>> Enable FPGA reconfiguration support on Stratix10 SoC.
>>>
>>> Signed-off-by: Chee Hong Ang 
>>> ---
>>>  drivers/fpga/Kconfig |  10 ++
>>>  drivers/fpga/Makefile|   1 +
>>>  drivers/fpga/stratix10.c | 298
>>> +++
>>>  3 files changed, 309 insertions(+)
>>>  create mode 100644 drivers/fpga/stratix10.c
>>>
>>> diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
>>> index d36c4c5..255683d 100644
>>> --- a/drivers/fpga/Kconfig
>>> +++ b/drivers/fpga/Kconfig
>>> @@ -31,6 +31,16 @@ config FPGA_CYCLON2
>>>       Enable FPGA driver for loading bitstream in BIT and BIN
>>> format
>>>       on Altera Cyclone II device.
>>>  
>>> +config FPGA_STRATIX10
>>> +   bool "Enable Altera FPGA driver for Stratix 10"
>>> +   depends on FPGA_ALTERA && TARGET_SOCFPGA_STRATIX10
>>> +   help
>>> +     Say Y here to enable the Altera Stratix 10 FPGA specific
>>> driver
>>> +
>>> +     This provides common functionality for Altera Stratix 10
>>> devices.
>>> +     Enable FPGA driver for writing bitstream into Altera
>>> Stratix10
>>> +     device.
>>> +
>>>  config FPGA_XILINX
>>>     bool "Enable Xilinx FPGA drivers"
>>>     select FPGA
>>> diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
>>> index 08c9ff8..3c906ec 100644
>>> --- a/drivers/fpga/Makefile
>>> +++ b/drivers/fpga/Makefile
>>> @@ -19,6 +19,7 @@ obj-$(CONFIG_FPGA_ACEX1K) += ACEX1K.o
>>>  obj-$(CONFIG_FPGA_CYCLON2) += cyclon2.o
>>>  obj-$(CONFIG_FPGA_STRATIX_II) += stratixII.o
>>>  obj-$(CONFIG_FPGA_STRATIX_V) += stratixv.o
>>> +obj-$(CONFIG_FPGA_STRATIX10) += stratix10.o
>>>  obj-$(CONFIG_FPGA_SOCFPGA) += socfpga.o
>>>  obj-$(CONFIG_TARGET_SOCFPGA_GEN5) += socfpga_gen5.o
>>>  obj-$(CONFIG_TARGET_SOCFPGA_ARRIA10) += socfpga_arria10.o
>>> diff --git a/drivers/fpga/stratix10.c b/drivers/fpga/stratix10.c
>>> new file mode 100644
>>> index 000..f0c5ace
>>> --- /dev/null
>>> +++ b/drivers/fpga/stratix10.c
>>> @@ -0,0 +1,298 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * Copyright (C) 2016-2018 Intel Corporation 
>>> + *
>>> + */
>>> +
>>> +#include 
>>> +#include 
>>> +#include 
>>> +
>>> +#define RECONFIG_STATUS_POLL_RESP_TIMEOUT_MS   6
>>> +#define RECONFIG_STATUS_INTERVAL_DELAY_US  100
>>> +
>>> +static const struct mbox_cfgstat_state {
>>> +   int err_no;
>>> +   const char  *error_name;
>>> +} mbox_cfgstat_state[] = {
>>> +   {MBOX_CFGSTAT_STATE_IDLE, "FPGA in idle mode."},
>>> +   {MBOX_CFGSTAT_STATE_CONFIG, "FPGA in config mode."},
>>> +   {MBOX_CFGSTAT_STATE_FAILACK, "Acknowledgement failed!"},
>>> +   {MBOX_CFGSTAT_STATE_ERROR_INVALID, "Invalid bitstream!"},
>>> +   {MBOX_CFGSTAT_STATE_ERROR_CORRUPT, "Corrupted
>>> bitstream!"},
>>> +   {MBOX_CFGSTAT_STATE_ERROR_AUTH, "Authentication failed!"},
>>> +   {MBOX_CFGSTAT_STATE_ERROR_CORE_IO, "I/O error!"},
>>> +   {MBOX_CFGSTAT_STATE_ERROR_HARDWARE, "Hardware error!"},
>>> +   {MBOX_CFGSTAT_STATE_ERROR_FAKE, "Fake error!"},
>>> +   {MBOX_CFGSTAT_STATE_ERROR_BOOT_INFO, "Error in boot
>>> info!"},
>>> +   {MBOX_CFGSTAT_STATE_ERROR_QSPI_ERROR, "Error in QSPI!"},
>>> +   {-ETIMEDOUT, "I/O timeout error"},
>>> +   {-1, "Unknown error!"}
>>> +};
>>> +
>>> +#define MBOX_CFGSTAT_MAX \
>>> +     (sizeof(mbox_cfgstat_state) / sizeof(struct
>>> mbox_cfgstat_state))
>>> +
>>> +static const char *mbox_cfgstat_to_str(int err)
>>> +{
>>> +   int i;
>>> +
>>> +   for (i = 0; i < MBOX_CFGSTAT_MAX - 1; i++) {
>>> +   if (mbox_cfgstat_state[i].err_no == err)
>>> +   return mbox_cfgstat_state[i].error_name;
>>> +   }
>>> +
>>> +   return mbox_cfgstat_state[MBOX_CFGSTAT_MAX -
>>> 1].error_name;
>>> +}
>>> +
>>> +static void inc_cmd_id(u8 *id)
>>> +{
>>> +   u8 val = *id + 1;
>>> +
>>> +   if (val > 15)
>>> +   val = 1;
>> What's this function about, elaborate implementation of (i % 15) + 1
>> ?
> This function increment the 4 bits transaction ID (1-15, 0 is unused)
> used for sending mailbox command to device.
>>
>>>
>>> +   *id = val;
>>> +}
>>> +
>>> +/*
>>> + * Polling the FPGA configuration status.
>>> + * Return 0 for success, non-zero for error.
>>> + */
>>> +static int reconfig_status_polling_resp(void)
>>> +{
>>> +   u32 reconfig_status_resp_len;
>>> +   u32 reconfig_status_resp[RECONFIG_STATUS_RESPONSE_LEN];
>>> +   int ret;
>>> +   unsigned long start = get_timer(0);
>>> +
>>> +   while (1) {
>>> +   reconfig_status_resp_len =
>>> RECONFIG_STATUS_RESPONSE_LEN;
>>> +   ret = mbox_send_cmd(MBOX_ID_UBOOT,
>>> MBOX_RECONFIG_STATUS,
>>> +   MBOX_CMD_DIRECT, 0, NULL, 0,
>>> +   _status_resp_len,
>>> +   reconfig_status_resp);

Re: [U-Boot] [PATCH v1 1/3] arm: socfpga: stratix10: Add Stratix10 FPGA Reconfiguration Driver

[Ang, Chee Hong]

On Fri, 2018-04-20 at 05:41 +0200, Marek Vasut wrote:
> On 04/20/2018 05:26 AM, chee.hong@intel.com wrote:
> > 
> > From: Chee Hong Ang 
> > 
> > Enable FPGA reconfiguration support on Stratix10 SoC.
> > 
> > Signed-off-by: Chee Hong Ang 
> > ---
> >  drivers/fpga/Kconfig |  10 ++
> >  drivers/fpga/Makefile|   1 +
> >  drivers/fpga/stratix10.c | 298
> > +++
> >  3 files changed, 309 insertions(+)
> >  create mode 100644 drivers/fpga/stratix10.c
> > 
> > diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
> > index d36c4c5..255683d 100644
> > --- a/drivers/fpga/Kconfig
> > +++ b/drivers/fpga/Kconfig
> > @@ -31,6 +31,16 @@ config FPGA_CYCLON2
> >       Enable FPGA driver for loading bitstream in BIT and BIN
> > format
> >       on Altera Cyclone II device.
> >  
> > +config FPGA_STRATIX10
> > +   bool "Enable Altera FPGA driver for Stratix 10"
> > +   depends on FPGA_ALTERA && TARGET_SOCFPGA_STRATIX10
> > +   help
> > +     Say Y here to enable the Altera Stratix 10 FPGA specific
> > driver
> > +
> > +     This provides common functionality for Altera Stratix 10
> > devices.
> > +     Enable FPGA driver for writing bitstream into Altera
> > Stratix10
> > +     device.
> > +
> >  config FPGA_XILINX
> >     bool "Enable Xilinx FPGA drivers"
> >     select FPGA
> > diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
> > index 08c9ff8..3c906ec 100644
> > --- a/drivers/fpga/Makefile
> > +++ b/drivers/fpga/Makefile
> > @@ -19,6 +19,7 @@ obj-$(CONFIG_FPGA_ACEX1K) += ACEX1K.o
> >  obj-$(CONFIG_FPGA_CYCLON2) += cyclon2.o
> >  obj-$(CONFIG_FPGA_STRATIX_II) += stratixII.o
> >  obj-$(CONFIG_FPGA_STRATIX_V) += stratixv.o
> > +obj-$(CONFIG_FPGA_STRATIX10) += stratix10.o
> >  obj-$(CONFIG_FPGA_SOCFPGA) += socfpga.o
> >  obj-$(CONFIG_TARGET_SOCFPGA_GEN5) += socfpga_gen5.o
> >  obj-$(CONFIG_TARGET_SOCFPGA_ARRIA10) += socfpga_arria10.o
> > diff --git a/drivers/fpga/stratix10.c b/drivers/fpga/stratix10.c
> > new file mode 100644
> > index 000..f0c5ace
> > --- /dev/null
> > +++ b/drivers/fpga/stratix10.c
> > @@ -0,0 +1,298 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2016-2018 Intel Corporation 
> > + *
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#define RECONFIG_STATUS_POLL_RESP_TIMEOUT_MS   6
> > +#define RECONFIG_STATUS_INTERVAL_DELAY_US  100
> > +
> > +static const struct mbox_cfgstat_state {
> > +   int err_no;
> > +   const char  *error_name;
> > +} mbox_cfgstat_state[] = {
> > +   {MBOX_CFGSTAT_STATE_IDLE, "FPGA in idle mode."},
> > +   {MBOX_CFGSTAT_STATE_CONFIG, "FPGA in config mode."},
> > +   {MBOX_CFGSTAT_STATE_FAILACK, "Acknowledgement failed!"},
> > +   {MBOX_CFGSTAT_STATE_ERROR_INVALID, "Invalid bitstream!"},
> > +   {MBOX_CFGSTAT_STATE_ERROR_CORRUPT, "Corrupted
> > bitstream!"},
> > +   {MBOX_CFGSTAT_STATE_ERROR_AUTH, "Authentication failed!"},
> > +   {MBOX_CFGSTAT_STATE_ERROR_CORE_IO, "I/O error!"},
> > +   {MBOX_CFGSTAT_STATE_ERROR_HARDWARE, "Hardware error!"},
> > +   {MBOX_CFGSTAT_STATE_ERROR_FAKE, "Fake error!"},
> > +   {MBOX_CFGSTAT_STATE_ERROR_BOOT_INFO, "Error in boot
> > info!"},
> > +   {MBOX_CFGSTAT_STATE_ERROR_QSPI_ERROR, "Error in QSPI!"},
> > +   {-ETIMEDOUT, "I/O timeout error"},
> > +   {-1, "Unknown error!"}
> > +};
> > +
> > +#define MBOX_CFGSTAT_MAX \
> > +     (sizeof(mbox_cfgstat_state) / sizeof(struct
> > mbox_cfgstat_state))
> > +
> > +static const char *mbox_cfgstat_to_str(int err)
> > +{
> > +   int i;
> > +
> > +   for (i = 0; i < MBOX_CFGSTAT_MAX - 1; i++) {
> > +   if (mbox_cfgstat_state[i].err_no == err)
> > +   return mbox_cfgstat_state[i].error_name;
> > +   }
> > +
> > +   return mbox_cfgstat_state[MBOX_CFGSTAT_MAX -
> > 1].error_name;
> > +}
> > +
> > +static void inc_cmd_id(u8 *id)
> > +{
> > +   u8 val = *id + 1;
> > +
> > +   if (val > 15)
> > +   val = 1;
> What's this function about, elaborate implementation of (i % 15) + 1
> ?
This function increment the 4 bits transaction ID (1-15, 0 is unused)
used for sending mailbox command to device.
> 
> > 
> > +   *id = val;
> > +}
> > +
> > +/*
> > + * Polling the FPGA configuration status.
> > + * Return 0 for success, non-zero for error.
> > + */
> > +static int reconfig_status_polling_resp(void)
> > +{
> > +   u32 reconfig_status_resp_len;
> > +   u32 reconfig_status_resp[RECONFIG_STATUS_RESPONSE_LEN];
> > +   int ret;
> > +   unsigned long start = get_timer(0);
> > +
> > +   while (1) {
> > +   reconfig_status_resp_len =
> > RECONFIG_STATUS_RESPONSE_LEN;
> > +   ret = mbox_send_cmd(MBOX_ID_UBOOT,
> > MBOX_RECONFIG_STATUS,
> > +   MBOX_CMD_DIRECT, 0, NULL, 0,
> > +   _status_resp_len,
> > +   reconfig_status_resp);
> > +
> > +   if (ret) {
> > +

Re: [U-Boot] [PATCH v1 1/3] arm: socfpga: stratix10: Add Stratix10 FPGA Reconfiguration Driver

[Marek Vasut]

On 04/20/2018 05:26 AM, chee.hong@intel.com wrote:
> From: Chee Hong Ang 
> 
> Enable FPGA reconfiguration support on Stratix10 SoC.
> 
> Signed-off-by: Chee Hong Ang 
> ---
>  drivers/fpga/Kconfig |  10 ++
>  drivers/fpga/Makefile|   1 +
>  drivers/fpga/stratix10.c | 298 
> +++
>  3 files changed, 309 insertions(+)
>  create mode 100644 drivers/fpga/stratix10.c
> 
> diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
> index d36c4c5..255683d 100644
> --- a/drivers/fpga/Kconfig
> +++ b/drivers/fpga/Kconfig
> @@ -31,6 +31,16 @@ config FPGA_CYCLON2
> Enable FPGA driver for loading bitstream in BIT and BIN format
> on Altera Cyclone II device.
>  
> +config FPGA_STRATIX10
> + bool "Enable Altera FPGA driver for Stratix 10"
> + depends on FPGA_ALTERA && TARGET_SOCFPGA_STRATIX10
> + help
> +   Say Y here to enable the Altera Stratix 10 FPGA specific driver
> +
> +   This provides common functionality for Altera Stratix 10 devices.
> +   Enable FPGA driver for writing bitstream into Altera Stratix10
> +   device.
> +
>  config FPGA_XILINX
>   bool "Enable Xilinx FPGA drivers"
>   select FPGA
> diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
> index 08c9ff8..3c906ec 100644
> --- a/drivers/fpga/Makefile
> +++ b/drivers/fpga/Makefile
> @@ -19,6 +19,7 @@ obj-$(CONFIG_FPGA_ACEX1K) += ACEX1K.o
>  obj-$(CONFIG_FPGA_CYCLON2) += cyclon2.o
>  obj-$(CONFIG_FPGA_STRATIX_II) += stratixII.o
>  obj-$(CONFIG_FPGA_STRATIX_V) += stratixv.o
> +obj-$(CONFIG_FPGA_STRATIX10) += stratix10.o
>  obj-$(CONFIG_FPGA_SOCFPGA) += socfpga.o
>  obj-$(CONFIG_TARGET_SOCFPGA_GEN5) += socfpga_gen5.o
>  obj-$(CONFIG_TARGET_SOCFPGA_ARRIA10) += socfpga_arria10.o
> diff --git a/drivers/fpga/stratix10.c b/drivers/fpga/stratix10.c
> new file mode 100644
> index 000..f0c5ace
> --- /dev/null
> +++ b/drivers/fpga/stratix10.c
> @@ -0,0 +1,298 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2016-2018 Intel Corporation 
> + *
> + */
> +
> +#include 
> +#include 
> +#include 
> +
> +#define RECONFIG_STATUS_POLL_RESP_TIMEOUT_MS 6
> +#define RECONFIG_STATUS_INTERVAL_DELAY_US100
> +
> +static const struct mbox_cfgstat_state {
> + int err_no;
> + const char  *error_name;
> +} mbox_cfgstat_state[] = {
> + {MBOX_CFGSTAT_STATE_IDLE, "FPGA in idle mode."},
> + {MBOX_CFGSTAT_STATE_CONFIG, "FPGA in config mode."},
> + {MBOX_CFGSTAT_STATE_FAILACK, "Acknowledgement failed!"},
> + {MBOX_CFGSTAT_STATE_ERROR_INVALID, "Invalid bitstream!"},
> + {MBOX_CFGSTAT_STATE_ERROR_CORRUPT, "Corrupted bitstream!"},
> + {MBOX_CFGSTAT_STATE_ERROR_AUTH, "Authentication failed!"},
> + {MBOX_CFGSTAT_STATE_ERROR_CORE_IO, "I/O error!"},
> + {MBOX_CFGSTAT_STATE_ERROR_HARDWARE, "Hardware error!"},
> + {MBOX_CFGSTAT_STATE_ERROR_FAKE, "Fake error!"},
> + {MBOX_CFGSTAT_STATE_ERROR_BOOT_INFO, "Error in boot info!"},
> + {MBOX_CFGSTAT_STATE_ERROR_QSPI_ERROR, "Error in QSPI!"},
> + {-ETIMEDOUT, "I/O timeout error"},
> + {-1, "Unknown error!"}
> +};
> +
> +#define MBOX_CFGSTAT_MAX \
> +   (sizeof(mbox_cfgstat_state) / sizeof(struct mbox_cfgstat_state))
> +
> +static const char *mbox_cfgstat_to_str(int err)
> +{
> + int i;
> +
> + for (i = 0; i < MBOX_CFGSTAT_MAX - 1; i++) {
> + if (mbox_cfgstat_state[i].err_no == err)
> + return mbox_cfgstat_state[i].error_name;
> + }
> +
> + return mbox_cfgstat_state[MBOX_CFGSTAT_MAX - 1].error_name;
> +}
> +
> +static void inc_cmd_id(u8 *id)
> +{
> + u8 val = *id + 1;
> +
> + if (val > 15)
> + val = 1;

What's this function about, elaborate implementation of (i % 15) + 1 ?

> + *id = val;
> +}
> +
> +/*
> + * Polling the FPGA configuration status.
> + * Return 0 for success, non-zero for error.
> + */
> +static int reconfig_status_polling_resp(void)
> +{
> + u32 reconfig_status_resp_len;
> + u32 reconfig_status_resp[RECONFIG_STATUS_RESPONSE_LEN];
> + int ret;
> + unsigned long start = get_timer(0);
> +
> + while (1) {
> + reconfig_status_resp_len = RECONFIG_STATUS_RESPONSE_LEN;
> + ret = mbox_send_cmd(MBOX_ID_UBOOT, MBOX_RECONFIG_STATUS,
> + MBOX_CMD_DIRECT, 0, NULL, 0,
> + _status_resp_len,
> + reconfig_status_resp);
> +
> + if (ret) {
> + puts("Failure in RECONFIG_STATUS mailbox command!\n");
> + return ret;
> + }
> +
> + /* Check for any error */
> + ret = reconfig_status_resp[RECONFIG_STATUS_STATE];
> + if (ret && ret != MBOX_CFGSTAT_STATE_CONFIG)
> + return ret;
> +
> + /* Make sure nStatus is not 0 */
> +