On Mon, Apr 01, 2024 at 10:09:05AM -0700, Russ Weight wrote:
> On Mon, Apr 01, 2024 at 05:46:29PM +0800, Xu Yilun wrote:
> > On Thu, Mar 28, 2024 at 07:35:59PM -0400, Peter Colberg wrote:
> > > From: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx>
> > >
> > > The size of the staging area in FLASH for FPGA updates is dependent on the
> > > size of the FPGA. Currently, the staging size is defined as a constant.
> > > Larger FPGAs are coming soon and it will soon be necessary to support
> >
> > Soon? When? You cannot add some feature without a user case. If you do
> > have a use case, put the patch in the same patchset.
>
> There may never be an up-streamed use-case. This is a very small
> change intended to make it easier for a third-party vendor to
> build a card that requires a larger staging area in FLASH. They
> would have to add a new "struct m10bmc_csr_map", but they
> wouldn't have to refactor this code as part of the change
>
> This change does not introduce an unused function or variable.
> It is more of a clean-up, making the code more flexible.
>
> Can it not be taken as is?
Would it be acceptable to just change the commit message to something
like:
Do not hardwire the staging size in the secure update driver. Move
the staging size to the m10bmc_csr_map structure to make the size
assignment more flexible.
>
> - Russ
>
> >
> > Thanks,
> > Yilun
> >
> > > different sizes for the staging area. Add a new staging_size member to the
> > > csr_map structure to support a variable staging size.
> > >
> > > The secure update driver does a sanity-check of the image size in
> > > comparison to the size of the staging area in FLASH. Change the
> > > staging size reference to a variable instead of a constant in order
> > > to more readily support future, larger FPGAs.
> > >
> > > Co-developed-by: Russ Weight <russell.h.weight@xxxxxxxxx>
> > > Signed-off-by: Russ Weight <russell.h.weight@xxxxxxxxx>
> > > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx>
> > > Signed-off-by: Peter Colberg <peter.colberg@xxxxxxxxx>
> > > ---
> > > drivers/fpga/intel-m10-bmc-sec-update.c | 3 ++-
> > > drivers/mfd/intel-m10-bmc-pmci.c | 1 +
> > > drivers/mfd/intel-m10-bmc-spi.c | 1 +
> > > include/linux/mfd/intel-m10-bmc.h | 1 +
> > > 4 files changed, 5 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/fpga/intel-m10-bmc-sec-update.c b/drivers/fpga/intel-m10-bmc-sec-update.c
> > > index 89851b133709..7ac9f9f5af12 100644
> > > --- a/drivers/fpga/intel-m10-bmc-sec-update.c
> > > +++ b/drivers/fpga/intel-m10-bmc-sec-update.c
> > > @@ -529,11 +529,12 @@ static enum fw_upload_err m10bmc_sec_prepare(struct fw_upload *fwl,
> > > const u8 *data, u32 size)
> > > {
> > > struct m10bmc_sec *sec = fwl->dd_handle;
> > > + const struct m10bmc_csr_map *csr_map = sec->m10bmc->info->csr_map;
> > > u32 ret;
> > >
> > > sec->cancel_request = false;
> > >
> > > - if (!size || size > M10BMC_STAGING_SIZE)
> > > + if (!size || size > csr_map->staging_size)
> > > return FW_UPLOAD_ERR_INVALID_SIZE;
> > >
> > > if (sec->m10bmc->flash_bulk_ops)
> > > diff --git a/drivers/mfd/intel-m10-bmc-pmci.c b/drivers/mfd/intel-m10-bmc-pmci.c
> > > index 0392ef8b57d8..698c5933938b 100644
> > > --- a/drivers/mfd/intel-m10-bmc-pmci.c
> > > +++ b/drivers/mfd/intel-m10-bmc-pmci.c
> > > @@ -370,6 +370,7 @@ static const struct m10bmc_csr_map m10bmc_n6000_csr_map = {
> > > .pr_reh_addr = M10BMC_N6000_PR_REH_ADDR,
> > > .pr_magic = M10BMC_N6000_PR_PROG_MAGIC,
> > > .rsu_update_counter = M10BMC_N6000_STAGING_FLASH_COUNT,
> > > + .staging_size = M10BMC_STAGING_SIZE,
> > > };
> > >
> > > static const struct intel_m10bmc_platform_info m10bmc_pmci_n6000 = {
> > > diff --git a/drivers/mfd/intel-m10-bmc-spi.c b/drivers/mfd/intel-m10-bmc-spi.c
> > > index cbeb7de9e041..d64d28199df6 100644
> > > --- a/drivers/mfd/intel-m10-bmc-spi.c
> > > +++ b/drivers/mfd/intel-m10-bmc-spi.c
> > > @@ -109,6 +109,7 @@ static const struct m10bmc_csr_map m10bmc_n3000_csr_map = {
> > > .pr_reh_addr = M10BMC_N3000_PR_REH_ADDR,
> > > .pr_magic = M10BMC_N3000_PR_PROG_MAGIC,
> > > .rsu_update_counter = M10BMC_N3000_STAGING_FLASH_COUNT,
> > > + .staging_size = M10BMC_STAGING_SIZE,
> > > };
> > >
> > > static struct mfd_cell m10bmc_d5005_subdevs[] = {
> > > diff --git a/include/linux/mfd/intel-m10-bmc.h b/include/linux/mfd/intel-m10-bmc.h
> > > index ee66c9751003..988f1cd90032 100644
> > > --- a/include/linux/mfd/intel-m10-bmc.h
> > > +++ b/include/linux/mfd/intel-m10-bmc.h
> > > @@ -205,6 +205,7 @@ struct m10bmc_csr_map {
> > > unsigned int pr_reh_addr;
> > > unsigned int pr_magic;
> > > unsigned int rsu_update_counter;
> > > + unsigned int staging_size;
> > > };
> > >
> > > /**
> > > --
> > > 2.44.0
> > >
> > >
