Re: [PATCH] mmc: arm_pl180: Limit data transfer to U16_MAX
+ @Simon
Best Regards,
*Lean Sheng Tan*
9elements GmbH, Kortumstraße 19-21, 44787 Bochum, Germany
Email: sheng@9elements.com
Phone: *+49 234 68 94 188 <+492346894188>*
Mobile: *+49 176 76 113842 <+4917676113842>*
Registered office: Bochum
Commercial register: Amtsgericht Bochum, HRB 17519
Management: Sebastian German, Eray Bazaar
Data protection information according to Art. 13 GDPR
<https://9elements.com/privacy>
On Mon 4. Mar 2024 at 16:39, Lean Sheng Tan wrote:
> Quick reminder:
> Can anyone help to review this?
> Thanks!
>
> Best Regards,
> *Lean Sheng Tan*
>
>
>
> 9elements GmbH, Kortumstraße 19-21, 44787 Bochum, Germany
> Email: sheng@9elements.com
> Phone: *+49 234 68 94 188 <+492346894188>*
> Mobile: *+49 176 76 113842 <+4917676113842>*
>
> Registered office: Bochum
> Commercial register: Amtsgericht Bochum, HRB 17519
> Management: Sebastian German, Eray Bazaar
>
> Data protection information according to Art. 13 GDPR
> <https://9elements.com/privacy>
>
>
> On Tue, 27 Feb 2024 at 22:02, wrote:
>
>> From: max
>>
>> Currently fetching files bigger that cause a data transfer greater than
>> U16_MAX fails.
>>
>> The reason is that the specification defines the datalength register
>> as a 16 bit wide register, but in u-boot it is used as if it is an
>> 32 bit register. Therefore values greater than U16_MAX cause an
>> infinite loop inside u-boot. U-boot expects to get more data from
>> interface/hardware then it will ever get and therefore inifintely waits
>> for more data that will never come.
>>
>> Signed-off-by: max
>> Cc: Peng Fan
>> Cc: Jaehoon Chung
>> ---
>> drivers/mmc/arm_pl180_mmci.c | 11 +++
>> 1 file changed, 11 insertions(+)
>>
>> diff --git a/drivers/mmc/arm_pl180_mmci.c b/drivers/mmc/arm_pl180_mmci.c
>> index 5cf5502ed5..af2f9a5a84 100644
>> --- a/drivers/mmc/arm_pl180_mmci.c
>> +++ b/drivers/mmc/arm_pl180_mmci.c
>> @@ -231,6 +231,7 @@ static int do_data_transfer(struct mmc *dev,
>> u32 blksz = 0;
>> u32 data_ctrl = 0;
>> u32 data_len = (u32) (data->blocks * data->blocksize);
>> + assert(data_len < U16_MAX); // should be ensured by
>> arm_pl180_get_b_max
>>
>> if (!host->version2) {
>> blksz = (ffs(data->blocksize) - 1);
>> @@ -358,6 +359,14 @@ static int host_set_ios(struct mmc *dev)
>> return 0;
>> }
>>
>> +static int arm_pl180_get_b_max(struct udevice *dev, void *dst, lbaint_t
>> blkcnt)
>> +{
>> + struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev);
>> + struct mmc *mmc = upriv->mmc;
>> +
>> + return U16_MAX / mmc->read_bl_len;
>> +}
>> +
>> #ifndef CONFIG_DM_MMC
>> /* MMC uses open drain drivers in the enumeration phase */
>> static int mmc_host_reset(struct mmc *dev)
>> @@ -373,6 +382,7 @@ static const struct mmc_ops arm_pl180_mmci_ops = {
>> .send_cmd = host_request,
>> .set_ios = host_set_ios,
>> .init = mmc_host_reset,
>> + .get_b_max = arm_pl180_get_b_max,
>> };
>>
>> /*
>> @@ -531,6 +541,7 @@ static const struct dm_mmc_ops arm_pl180_dm_mmc_ops =
>> {
>> .send_cmd = dm_host_request,
>> .set_ios = dm_host_set_ios,
>> .get_cd = dm_mmc_getcd,
>> + .get_b_max = arm_pl180_get_b_max,
>> };
>>
>> static int arm_pl180_mmc_of_to_plat(struct udevice *dev)
>> --
>> 2.43.0
>>
>>
Re: [PATCH] mmc: arm_pl180: Limit data transfer to U16_MAX
Quick reminder:
Can anyone help to review this?
Thanks!
Best Regards,
*Lean Sheng Tan*
9elements GmbH, Kortumstraße 19-21, 44787 Bochum, Germany
Email: sheng@9elements.com
Phone: *+49 234 68 94 188 <+492346894188>*
Mobile: *+49 176 76 113842 <+4917676113842>*
Registered office: Bochum
Commercial register: Amtsgericht Bochum, HRB 17519
Management: Sebastian German, Eray Bazaar
Data protection information according to Art. 13 GDPR
<https://9elements.com/privacy>
On Tue, 27 Feb 2024 at 22:02, wrote:
> From: max
>
> Currently fetching files bigger that cause a data transfer greater than
> U16_MAX fails.
>
> The reason is that the specification defines the datalength register
> as a 16 bit wide register, but in u-boot it is used as if it is an
> 32 bit register. Therefore values greater than U16_MAX cause an
> infinite loop inside u-boot. U-boot expects to get more data from
> interface/hardware then it will ever get and therefore inifintely waits
> for more data that will never come.
>
> Signed-off-by: max
> Cc: Peng Fan
> Cc: Jaehoon Chung
> ---
> drivers/mmc/arm_pl180_mmci.c | 11 +++
> 1 file changed, 11 insertions(+)
>
> diff --git a/drivers/mmc/arm_pl180_mmci.c b/drivers/mmc/arm_pl180_mmci.c
> index 5cf5502ed5..af2f9a5a84 100644
> --- a/drivers/mmc/arm_pl180_mmci.c
> +++ b/drivers/mmc/arm_pl180_mmci.c
> @@ -231,6 +231,7 @@ static int do_data_transfer(struct mmc *dev,
> u32 blksz = 0;
> u32 data_ctrl = 0;
> u32 data_len = (u32) (data->blocks * data->blocksize);
> + assert(data_len < U16_MAX); // should be ensured by
> arm_pl180_get_b_max
>
> if (!host->version2) {
> blksz = (ffs(data->blocksize) - 1);
> @@ -358,6 +359,14 @@ static int host_set_ios(struct mmc *dev)
> return 0;
> }
>
> +static int arm_pl180_get_b_max(struct udevice *dev, void *dst, lbaint_t
> blkcnt)
> +{
> + struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev);
> + struct mmc *mmc = upriv->mmc;
> +
> + return U16_MAX / mmc->read_bl_len;
> +}
> +
> #ifndef CONFIG_DM_MMC
> /* MMC uses open drain drivers in the enumeration phase */
> static int mmc_host_reset(struct mmc *dev)
> @@ -373,6 +382,7 @@ static const struct mmc_ops arm_pl180_mmci_ops = {
> .send_cmd = host_request,
> .set_ios = host_set_ios,
> .init = mmc_host_reset,
> + .get_b_max = arm_pl180_get_b_max,
> };
>
> /*
> @@ -531,6 +541,7 @@ static const struct dm_mmc_ops arm_pl180_dm_mmc_ops = {
> .send_cmd = dm_host_request,
> .set_ios = dm_host_set_ios,
> .get_cd = dm_mmc_getcd,
> + .get_b_max = arm_pl180_get_b_max,
> };
>
> static int arm_pl180_mmc_of_to_plat(struct udevice *dev)
> --
> 2.43.0
>
>
Re: [PATCH v7 2/2] schemas: Add some common reserved-memory usages
Hi Rob, Sure, I will ask if the edk2 developers who work together for UPL spec could help to respond here. Hi Ard, Regarding this part: *However, there is no scoping in DT schema as far as I am aware, which means that the OS may be forced/expected to interpret these regions beyond simply disregarding them and treating them as reserved memory, and *that* is something I strongly object to.* I think that is one of good perspective to look into this, however please also consider this situation: that everyone is just starting to develop their own DT schemas (e.g. RISC-V), and there is no way to stop OS to also pick up those DT nodes. in that case, if we do not maintain a more unified DT schema, then OS might end up with more conflicting and possibly contracting DT nodes/ properties (e.g. RISC-V with own DT schemas using U-Boot (another DT schema) + edk2 payload (another DT schema, like UPL) to boot to OS. So personally I still prefer a unified DT schema, even if the OS never uses them, but that would be very beneficial and in control in the long term if more people are using DT. For now, the DT does serve as the purpose of communication vehicle in between platform init and payload, which is still within Firmware stack. However from edk2 stand point, there is more people want to roll out DT for more internal usage within edk2 itself, for example: https://uefi.org/sites/default/files/resources/Embracing%20Modularity%20and%20Boot-Time%20Configuration%20Faster%20TTM%20with%20Tiano-based%20Solutions_Warkentin.pdf For sure, it is much easier for us (and time saving as well) to just maintain DT schema/ format within our own UPL spec, but as mentioned, for better long term maintenance and community collaborations, we decided to upstream our implementation back to the main DT schema :) Thanks! Best Regards, *Lean Sheng Tan* 9elements GmbH, Kortumstraße 19-21, 44787 Bochum, Germany Email: sheng@9elements.com Phone: *+49 234 68 94 188 <+492346894188>* Mobile: *+49 176 76 113842 <+4917676113842>* Registered office: Bochum Commercial register: Amtsgericht Bochum, HRB 17519 Management: Sebastian German, Eray Bazaar Data protection information according to Art. 13 GDPR <https://9elements.com/privacy> On Wed, 8 Nov 2023 at 15:20, Ard Biesheuvel wrote: > On Wed, 8 Nov 2023 at 14:57, Rob Herring wrote: > > > > On Wed, Nov 8, 2023 at 5:38 AM Ard Biesheuvel wrote: > > > > > > On Tue, 7 Nov 2023 at 19:07, Rob Herring wrote: > > > > > > > > > > > > All of this: > > > > > > > > > > > > On Mon, 16 Oct 2023 at 15:54, Simon Glass > wrote: > > > > > > > > > > > > It is not specific to EDK2. Imagine this boot sequence: > > > > > > > > > > > > - Platform Init (U-Boot) starts up > > > > > > - U-Boot uses its platform knowledge to sets some ACPI tables > and put > > > > > > various things in memory > > > > > > - U-Boot sets up some runtime code and data for the OS > > > > > > - U-Boot jumps to the Tianocore payload ** > > > > > > - Payload (Tianocore) wants to know where the ACPI tables are, > for example > > > > > > - Tianocore needs to provide boot services to the OS, so needs > to know > > > > > > the memory map, etc. > > > > > > > > > > > > ** At this point we want to use DT to pass the required > information. > > > > > > > > > > > > Of course, Platform Init could be coreboot or Tianocore or some > > > > > > strange private binary. Payload could be U-Boot or something > else. > > > > > > That is the point of this effort, to build interoperability. > > > > > > > > [...] > > > > > > > > > > Perhaps the problem here is that Linux has tied itself up in > knots > > > > > > with its EFI stuff and DT fixups and what-not. But this is not > that. > > > > > > It is a simple handoff between two pieces of firmware, Platform > Init > > > > > > and Payload. It has nothing to do with the OS. With Tianocore > they are > > > > > > typically combined, but with this usage they are split, and we > can > > > > > > swap out one project for another on either side of the DT > interface. > > > > > > > > Is perhaps the clearest description of the problem you want to solve. > > > > It's clearly related to EFI though not the interface to the OS. IIRC, > > > > "platform init" and "payload" are terms in the UEFI spec, right? > > > > > > No they are not. Thi
Re: [PATCH v4 4/4] memory: Add ECC property
Hi Rob, Sorry for missing this: regarding your question on whether if the memory can support both single-bit and multi-bit ECC, i think the answer is yes. @Dong, Guo or @Chiu, Chasel could you help to confirm on this? Thanks. Best Regards, *Lean Sheng Tan* 9elements GmbH, Kortumstraße 19-21, 44787 Bochum, Germany Email: sheng@9elements.com Phone: *+49 234 68 94 188 <+492346894188>* Mobile: *+49 176 76 113842 <+4917676113842>* Registered office: Bochum Commercial register: Amtsgericht Bochum, HRB 17519 Management: Sebastian German, Eray Bazaar Data protection information according to Art. 13 GDPR <https://9elements.com/privacy> On Tue, 29 Aug 2023 at 23:38, Rob Herring wrote: > On Tue, Aug 29, 2023 at 2:18 PM Simon Glass wrote: > > > > Some memories provides ECC correction. For software which wants to check > > memory, it is helpful to see which regions provide this feature. > > > > Add this as a property of the /memory nodes, since it presumably follows > > the hardware-level memory system. > > > > Signed-off-by: Simon Glass > > --- > > > > (no changes since v3) > > > > Changes in v3: > > - Add new patch to update the /memory nodes > > > > dtschema/schemas/memory.yaml | 9 - > > 1 file changed, 8 insertions(+), 1 deletion(-) > > > > diff --git a/dtschema/schemas/memory.yaml b/dtschema/schemas/memory.yaml > > index 1d74410..981af04 100644 > > --- a/dtschema/schemas/memory.yaml > > +++ b/dtschema/schemas/memory.yaml > > @@ -34,7 +34,14 @@ patternProperties: > > description: > >For the purpose of identification, each NUMA node is > associated with > >a unique token known as a node id. > > - > > + attr: > > Kind of vague. > > > +$ref: /schemas/types.yaml#/definitions/string-array > > +description: | > > + Attributes possessed by this memory region: > > + > > +"single-bit-ecc" - supports single-bit ECC > > +"multi-bit-ecc" - supports multiple-bit ECC > > "supports" means corrects or reports? Most h/w supports both, but only > reports multi-bit errors. > > > +"no-ecc" - non-ECC memory > > Don't define values in free form text. > > This form is difficult to validate especially when non-ECC related > attr's are added to the mix as we can't really define which > combinations are valid. For example how do we prevent: > > attr = "single-bit-ecc", "multi-bit-ecc"; > > Or maybe that's valid? If so, how would we express that? > > Why do we need "no-ecc"? Is that the same as no "attr" property? > > I think it's better if we have 'ecc-type' or something? Or generally, > a property per class/type of attribute. > > Rob >
