2023-12-15 05:21:24

by Ghanshyam Agrawal

[permalink] [raw]
Subject: [PATCH] gpu: drm: amd: fixed typos

Fixed multiple typos in atomfirmware.h

Signed-off-by: Ghanshyam Agrawal <[email protected]>
---
drivers/gpu/drm/amd/include/atomfirmware.h | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/include/atomfirmware.h b/drivers/gpu/drm/amd/include/atomfirmware.h
index fa7d6ced786f..41d553921adf 100644
--- a/drivers/gpu/drm/amd/include/atomfirmware.h
+++ b/drivers/gpu/drm/amd/include/atomfirmware.h
@@ -210,7 +210,7 @@ atom_bios_string = "ATOM"
};
*/

-#pragma pack(1) /* BIOS data must use byte aligment*/
+#pragma pack(1) /* BIOS data must use byte alignment */

enum atombios_image_offset{
OFFSET_TO_ATOM_ROM_HEADER_POINTER = 0x00000048,
@@ -452,7 +452,7 @@ struct atom_dtd_format
uint8_t refreshrate;
};

-/* atom_dtd_format.modemiscinfo defintion */
+/* atom_dtd_format.modemiscinfo definition */
enum atom_dtd_format_modemiscinfo{
ATOM_HSYNC_POLARITY = 0x0002,
ATOM_VSYNC_POLARITY = 0x0004,
@@ -645,7 +645,7 @@ struct lcd_info_v2_1
uint32_t reserved1[8];
};

-/* lcd_info_v2_1.panel_misc defintion */
+/* lcd_info_v2_1.panel_misc definition */
enum atom_lcd_info_panel_misc{
ATOM_PANEL_MISC_FPDI =0x0002,
};
@@ -683,7 +683,7 @@ enum atom_gpio_pin_assignment_gpio_id {
/* gpio_id pre-define id for multiple usage */
/* GPIO use to control PCIE_VDDC in certain SLT board */
PCIE_VDDC_CONTROL_GPIO_PINID = 56,
- /* if PP_AC_DC_SWITCH_GPIO_PINID in Gpio_Pin_LutTable, AC/DC swithing feature is enable */
+ /* if PP_AC_DC_SWITCH_GPIO_PINID in Gpio_Pin_LutTable, AC/DC switching feature is enable */
PP_AC_DC_SWITCH_GPIO_PINID = 60,
/* VDDC_REGULATOR_VRHOT_GPIO_PINID in Gpio_Pin_LutTable, VRHot feature is enable */
VDDC_VRHOT_GPIO_PINID = 61,
--
2.25.1



2023-12-15 05:29:45

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH] gpu: drm: amd: fixed typos

Hi--

On 12/14/23 21:20, Ghanshyam Agrawal wrote:
> Fixed multiple typos in atomfirmware.h
>
> Signed-off-by: Ghanshyam Agrawal <[email protected]>
> ---
> drivers/gpu/drm/amd/include/atomfirmware.h | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/include/atomfirmware.h b/drivers/gpu/drm/amd/include/atomfirmware.h
> index fa7d6ced786f..41d553921adf 100644
> --- a/drivers/gpu/drm/amd/include/atomfirmware.h
> +++ b/drivers/gpu/drm/amd/include/atomfirmware.h
> @@ -210,7 +210,7 @@ atom_bios_string = "ATOM"
> };
> */
>
> -#pragma pack(1) /* BIOS data must use byte aligment*/
> +#pragma pack(1) /* BIOS data must use byte alignment */
>
> enum atombios_image_offset{
> OFFSET_TO_ATOM_ROM_HEADER_POINTER = 0x00000048,
> @@ -452,7 +452,7 @@ struct atom_dtd_format
> uint8_t refreshrate;
> };
>
> -/* atom_dtd_format.modemiscinfo defintion */
> +/* atom_dtd_format.modemiscinfo definition */
> enum atom_dtd_format_modemiscinfo{
> ATOM_HSYNC_POLARITY = 0x0002,
> ATOM_VSYNC_POLARITY = 0x0004,
> @@ -645,7 +645,7 @@ struct lcd_info_v2_1
> uint32_t reserved1[8];
> };
>
> -/* lcd_info_v2_1.panel_misc defintion */
> +/* lcd_info_v2_1.panel_misc definition */
> enum atom_lcd_info_panel_misc{
> ATOM_PANEL_MISC_FPDI =0x0002,
> };
> @@ -683,7 +683,7 @@ enum atom_gpio_pin_assignment_gpio_id {
> /* gpio_id pre-define id for multiple usage */
> /* GPIO use to control PCIE_VDDC in certain SLT board */
> PCIE_VDDC_CONTROL_GPIO_PINID = 56,
> - /* if PP_AC_DC_SWITCH_GPIO_PINID in Gpio_Pin_LutTable, AC/DC swithing feature is enable */
> + /* if PP_AC_DC_SWITCH_GPIO_PINID in Gpio_Pin_LutTable, AC/DC switching feature is enable */

s/enable/enabled/

> PP_AC_DC_SWITCH_GPIO_PINID = 60,
> /* VDDC_REGULATOR_VRHOT_GPIO_PINID in Gpio_Pin_LutTable, VRHot feature is enable */

Ditto.
There may be a few more that need this same treatment.

> VDDC_VRHOT_GPIO_PINID = 61,

The other changes look good as far as they go, but codespell reports
a few more misspellings to consider:

atomfirmware.h:213: aligment ==> alignment
atomfirmware.h:257: Offest ==> Offset
atomfirmware.h:258: Offest ==> Offset
atomfirmware.h:390: Offest ==> Offset
atomfirmware.h:455: defintion ==> definition
atomfirmware.h:648: defintion ==> definition
atomfirmware.h:686: swithing ==> switching
atomfirmware.h:704: calcualted ==> calculated
atomfirmware.h:967: compability ==> compatibility
atomfirmware.h:981: intenal ==> internal
atomfirmware.h:993: intenal ==> internal
atomfirmware.h:3469: sequece ==> sequence
atomfirmware.h:3507: indiate ==> indicate
atomfirmware.h:4429: stucture ==> structure
atomfirmware.h:4430: stucture ==> structure
atomfirmware.h:4462: regiser ==> register


thanks.
--
#Randy
https://people.kernel.org/tglx/notes-about-netiquette
https://subspace.kernel.org/etiquette.html

2023-12-15 06:55:21

by Ghanshyam Agrawal

[permalink] [raw]
Subject: Re: [PATCH] gpu: drm: amd: fixed typos

On Fri, Dec 15, 2023 at 10:59 AM Randy Dunlap <[email protected]> wrote:
>
> Hi--
>
> On 12/14/23 21:20, Ghanshyam Agrawal wrote:
> > Fixed multiple typos in atomfirmware.h
> >
> > Signed-off-by: Ghanshyam Agrawal <[email protected]>
> > ---
> > drivers/gpu/drm/amd/include/atomfirmware.h | 8 ++++----
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/include/atomfirmware.h b/drivers/gpu/drm/amd/include/atomfirmware.h
> > index fa7d6ced786f..41d553921adf 100644
> > --- a/drivers/gpu/drm/amd/include/atomfirmware.h
> > +++ b/drivers/gpu/drm/amd/include/atomfirmware.h
> > @@ -210,7 +210,7 @@ atom_bios_string = "ATOM"
> > };
> > */
> >
> > -#pragma pack(1) /* BIOS data must use byte aligment*/
> > +#pragma pack(1) /* BIOS data must use byte alignment */
> >
> > enum atombios_image_offset{
> > OFFSET_TO_ATOM_ROM_HEADER_POINTER = 0x00000048,
> > @@ -452,7 +452,7 @@ struct atom_dtd_format
> > uint8_t refreshrate;
> > };
> >
> > -/* atom_dtd_format.modemiscinfo defintion */
> > +/* atom_dtd_format.modemiscinfo definition */
> > enum atom_dtd_format_modemiscinfo{
> > ATOM_HSYNC_POLARITY = 0x0002,
> > ATOM_VSYNC_POLARITY = 0x0004,
> > @@ -645,7 +645,7 @@ struct lcd_info_v2_1
> > uint32_t reserved1[8];
> > };
> >
> > -/* lcd_info_v2_1.panel_misc defintion */
> > +/* lcd_info_v2_1.panel_misc definition */
> > enum atom_lcd_info_panel_misc{
> > ATOM_PANEL_MISC_FPDI =0x0002,
> > };
> > @@ -683,7 +683,7 @@ enum atom_gpio_pin_assignment_gpio_id {
> > /* gpio_id pre-define id for multiple usage */
> > /* GPIO use to control PCIE_VDDC in certain SLT board */
> > PCIE_VDDC_CONTROL_GPIO_PINID = 56,
> > - /* if PP_AC_DC_SWITCH_GPIO_PINID in Gpio_Pin_LutTable, AC/DC swithing feature is enable */
> > + /* if PP_AC_DC_SWITCH_GPIO_PINID in Gpio_Pin_LutTable, AC/DC switching feature is enable */
>
> s/enable/enabled/
>
> > PP_AC_DC_SWITCH_GPIO_PINID = 60,
> > /* VDDC_REGULATOR_VRHOT_GPIO_PINID in Gpio_Pin_LutTable, VRHot feature is enable */
>
> Ditto.
> There may be a few more that need this same treatment.
>
> > VDDC_VRHOT_GPIO_PINID = 61,
>
> The other changes look good as far as they go, but codespell reports
> a few more misspellings to consider:
>
> atomfirmware.h:213: aligment ==> alignment
> atomfirmware.h:257: Offest ==> Offset
> atomfirmware.h:258: Offest ==> Offset
> atomfirmware.h:390: Offest ==> Offset
> atomfirmware.h:455: defintion ==> definition
> atomfirmware.h:648: defintion ==> definition
> atomfirmware.h:686: swithing ==> switching
> atomfirmware.h:704: calcualted ==> calculated
> atomfirmware.h:967: compability ==> compatibility
> atomfirmware.h:981: intenal ==> internal
> atomfirmware.h:993: intenal ==> internal
> atomfirmware.h:3469: sequece ==> sequence
> atomfirmware.h:3507: indiate ==> indicate
> atomfirmware.h:4429: stucture ==> structure
> atomfirmware.h:4430: stucture ==> structure
> atomfirmware.h:4462: regiser ==> register
>
>
> thanks.
> --
> #Randy
> https://people.kernel.org/tglx/notes-about-netiquette
> https://subspace.kernel.org/etiquette.html

Hi Randy,

Thanks for your feedback. I will correct the grammatical errors.

Regarding the other codespell suggestions, if I make the changes
then checkpatch script gives a lot of errors and warnings. Some
are related to usage of tabs, line lengths etc. Being a beginner
in the linux kernel development, I am not sure how to fix
(or whether to ignore) those warnings. Would it be okay if I
proceed with only the small number of changes I have suggested
with this patch itself?

Thanks & Regards,
Ghanshyam Agrawal

2023-12-15 15:58:51

by Alex Deucher

[permalink] [raw]
Subject: Re: [PATCH] gpu: drm: amd: fixed typos

On Fri, Dec 15, 2023 at 3:40 AM Ghanshyam Agrawal
<[email protected]> wrote:
>
> On Fri, Dec 15, 2023 at 10:59 AM Randy Dunlap <[email protected]> wrote:
> >
> > Hi--
> >
> > On 12/14/23 21:20, Ghanshyam Agrawal wrote:
> > > Fixed multiple typos in atomfirmware.h
> > >
> > > Signed-off-by: Ghanshyam Agrawal <[email protected]>
> > > ---
> > > drivers/gpu/drm/amd/include/atomfirmware.h | 8 ++++----
> > > 1 file changed, 4 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/amd/include/atomfirmware.h b/drivers/gpu/drm/amd/include/atomfirmware.h
> > > index fa7d6ced786f..41d553921adf 100644
> > > --- a/drivers/gpu/drm/amd/include/atomfirmware.h
> > > +++ b/drivers/gpu/drm/amd/include/atomfirmware.h
> > > @@ -210,7 +210,7 @@ atom_bios_string = "ATOM"
> > > };
> > > */
> > >
> > > -#pragma pack(1) /* BIOS data must use byte aligment*/
> > > +#pragma pack(1) /* BIOS data must use byte alignment */
> > >
> > > enum atombios_image_offset{
> > > OFFSET_TO_ATOM_ROM_HEADER_POINTER = 0x00000048,
> > > @@ -452,7 +452,7 @@ struct atom_dtd_format
> > > uint8_t refreshrate;
> > > };
> > >
> > > -/* atom_dtd_format.modemiscinfo defintion */
> > > +/* atom_dtd_format.modemiscinfo definition */
> > > enum atom_dtd_format_modemiscinfo{
> > > ATOM_HSYNC_POLARITY = 0x0002,
> > > ATOM_VSYNC_POLARITY = 0x0004,
> > > @@ -645,7 +645,7 @@ struct lcd_info_v2_1
> > > uint32_t reserved1[8];
> > > };
> > >
> > > -/* lcd_info_v2_1.panel_misc defintion */
> > > +/* lcd_info_v2_1.panel_misc definition */
> > > enum atom_lcd_info_panel_misc{
> > > ATOM_PANEL_MISC_FPDI =0x0002,
> > > };
> > > @@ -683,7 +683,7 @@ enum atom_gpio_pin_assignment_gpio_id {
> > > /* gpio_id pre-define id for multiple usage */
> > > /* GPIO use to control PCIE_VDDC in certain SLT board */
> > > PCIE_VDDC_CONTROL_GPIO_PINID = 56,
> > > - /* if PP_AC_DC_SWITCH_GPIO_PINID in Gpio_Pin_LutTable, AC/DC swithing feature is enable */
> > > + /* if PP_AC_DC_SWITCH_GPIO_PINID in Gpio_Pin_LutTable, AC/DC switching feature is enable */
> >
> > s/enable/enabled/
> >
> > > PP_AC_DC_SWITCH_GPIO_PINID = 60,
> > > /* VDDC_REGULATOR_VRHOT_GPIO_PINID in Gpio_Pin_LutTable, VRHot feature is enable */
> >
> > Ditto.
> > There may be a few more that need this same treatment.
> >
> > > VDDC_VRHOT_GPIO_PINID = 61,
> >
> > The other changes look good as far as they go, but codespell reports
> > a few more misspellings to consider:
> >
> > atomfirmware.h:213: aligment ==> alignment
> > atomfirmware.h:257: Offest ==> Offset
> > atomfirmware.h:258: Offest ==> Offset
> > atomfirmware.h:390: Offest ==> Offset
> > atomfirmware.h:455: defintion ==> definition
> > atomfirmware.h:648: defintion ==> definition
> > atomfirmware.h:686: swithing ==> switching
> > atomfirmware.h:704: calcualted ==> calculated
> > atomfirmware.h:967: compability ==> compatibility
> > atomfirmware.h:981: intenal ==> internal
> > atomfirmware.h:993: intenal ==> internal
> > atomfirmware.h:3469: sequece ==> sequence
> > atomfirmware.h:3507: indiate ==> indicate
> > atomfirmware.h:4429: stucture ==> structure
> > atomfirmware.h:4430: stucture ==> structure
> > atomfirmware.h:4462: regiser ==> register
> >
> >
> > thanks.
> > --
> > #Randy
> > https://people.kernel.org/tglx/notes-about-netiquette
> > https://subspace.kernel.org/etiquette.html
>
> Hi Randy,
>
> Thanks for your feedback. I will correct the grammatical errors.
>
> Regarding the other codespell suggestions, if I make the changes
> then checkpatch script gives a lot of errors and warnings. Some
> are related to usage of tabs, line lengths etc. Being a beginner
> in the linux kernel development, I am not sure how to fix
> (or whether to ignore) those warnings. Would it be okay if I
> proceed with only the small number of changes I have suggested
> with this patch itself?

How concerned are you with fixing these? This file is owned by
firmware teams within AMD and shared across different OSes. It
defines the structures present in the ROM on the board. I'm inclined
to just leave it be to avoid churn when syncing it with the canonical
source. We can certainly try and push the fixes back up into the
canonical code as well, but that may not be the easiest process.

Alex

2023-12-16 06:29:42

by Ghanshyam Agrawal

[permalink] [raw]
Subject: Re: [PATCH] gpu: drm: amd: fixed typos

On Fri, Dec 15, 2023 at 9:28 PM Alex Deucher <[email protected]> wrote:
>
> On Fri, Dec 15, 2023 at 3:40 AM Ghanshyam Agrawal
> <[email protected]> wrote:
> >
> > On Fri, Dec 15, 2023 at 10:59 AM Randy Dunlap <[email protected]> wrote:
> > >
> > > Hi--
> > >
> > > On 12/14/23 21:20, Ghanshyam Agrawal wrote:
> > > > Fixed multiple typos in atomfirmware.h
> > > >
> > > > Signed-off-by: Ghanshyam Agrawal <[email protected]>
> > > > ---
> > > > drivers/gpu/drm/amd/include/atomfirmware.h | 8 ++++----
> > > > 1 file changed, 4 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/amd/include/atomfirmware.h b/drivers/gpu/drm/amd/include/atomfirmware.h
> > > > index fa7d6ced786f..41d553921adf 100644
> > > > --- a/drivers/gpu/drm/amd/include/atomfirmware.h
> > > > +++ b/drivers/gpu/drm/amd/include/atomfirmware.h
> > > > @@ -210,7 +210,7 @@ atom_bios_string = "ATOM"
> > > > };
> > > > */
> > > >
> > > > -#pragma pack(1) /* BIOS data must use byte aligment*/
> > > > +#pragma pack(1) /* BIOS data must use byte alignment */
> > > >
> > > > enum atombios_image_offset{
> > > > OFFSET_TO_ATOM_ROM_HEADER_POINTER = 0x00000048,
> > > > @@ -452,7 +452,7 @@ struct atom_dtd_format
> > > > uint8_t refreshrate;
> > > > };
> > > >
> > > > -/* atom_dtd_format.modemiscinfo defintion */
> > > > +/* atom_dtd_format.modemiscinfo definition */
> > > > enum atom_dtd_format_modemiscinfo{
> > > > ATOM_HSYNC_POLARITY = 0x0002,
> > > > ATOM_VSYNC_POLARITY = 0x0004,
> > > > @@ -645,7 +645,7 @@ struct lcd_info_v2_1
> > > > uint32_t reserved1[8];
> > > > };
> > > >
> > > > -/* lcd_info_v2_1.panel_misc defintion */
> > > > +/* lcd_info_v2_1.panel_misc definition */
> > > > enum atom_lcd_info_panel_misc{
> > > > ATOM_PANEL_MISC_FPDI =0x0002,
> > > > };
> > > > @@ -683,7 +683,7 @@ enum atom_gpio_pin_assignment_gpio_id {
> > > > /* gpio_id pre-define id for multiple usage */
> > > > /* GPIO use to control PCIE_VDDC in certain SLT board */
> > > > PCIE_VDDC_CONTROL_GPIO_PINID = 56,
> > > > - /* if PP_AC_DC_SWITCH_GPIO_PINID in Gpio_Pin_LutTable, AC/DC swithing feature is enable */
> > > > + /* if PP_AC_DC_SWITCH_GPIO_PINID in Gpio_Pin_LutTable, AC/DC switching feature is enable */
> > >
> > > s/enable/enabled/
> > >
> > > > PP_AC_DC_SWITCH_GPIO_PINID = 60,
> > > > /* VDDC_REGULATOR_VRHOT_GPIO_PINID in Gpio_Pin_LutTable, VRHot feature is enable */
> > >
> > > Ditto.
> > > There may be a few more that need this same treatment.
> > >
> > > > VDDC_VRHOT_GPIO_PINID = 61,
> > >
> > > The other changes look good as far as they go, but codespell reports
> > > a few more misspellings to consider:
> > >
> > > atomfirmware.h:213: aligment ==> alignment
> > > atomfirmware.h:257: Offest ==> Offset
> > > atomfirmware.h:258: Offest ==> Offset
> > > atomfirmware.h:390: Offest ==> Offset
> > > atomfirmware.h:455: defintion ==> definition
> > > atomfirmware.h:648: defintion ==> definition
> > > atomfirmware.h:686: swithing ==> switching
> > > atomfirmware.h:704: calcualted ==> calculated
> > > atomfirmware.h:967: compability ==> compatibility
> > > atomfirmware.h:981: intenal ==> internal
> > > atomfirmware.h:993: intenal ==> internal
> > > atomfirmware.h:3469: sequece ==> sequence
> > > atomfirmware.h:3507: indiate ==> indicate
> > > atomfirmware.h:4429: stucture ==> structure
> > > atomfirmware.h:4430: stucture ==> structure
> > > atomfirmware.h:4462: regiser ==> register
> > >
> > >
> > > thanks.
> > > --
> > > #Randy
> > > https://people.kernel.org/tglx/notes-about-netiquette
> > > https://subspace.kernel.org/etiquette.html
> >
> > Hi Randy,
> >
> > Thanks for your feedback. I will correct the grammatical errors.
> >
> > Regarding the other codespell suggestions, if I make the changes
> > then checkpatch script gives a lot of errors and warnings. Some
> > are related to usage of tabs, line lengths etc. Being a beginner
> > in the linux kernel development, I am not sure how to fix
> > (or whether to ignore) those warnings. Would it be okay if I
> > proceed with only the small number of changes I have suggested
> > with this patch itself?
>
> How concerned are you with fixing these? This file is owned by
> firmware teams within AMD and shared across different OSes. It
> defines the structures present in the ROM on the board. I'm inclined
> to just leave it be to avoid churn when syncing it with the canonical
> source. We can certainly try and push the fixes back up into the
> canonical code as well, but that may not be the easiest process.
>
> Alex

Hi Alex,

Thanks for your review. I wanted to fix the typos because it helps
someone while searching through the code using text/ code searching
tools. If it is causing trouble as you suggested, we may as well
leave it as is. Let the maintainers take a call on this.

Thanks & Regards,
Ghanshyam Agrawal