Re: [flashrom] [PATCH] Fix wrong density encoding on Intel Silvermont.

[Tai-Hong (Type) Wu]

> > I will probably commit this patch before you get a chance to test it
> > again because I want to do the 0.9.8 release soonish, but I'd
> > appreciate a later test very much too if you want to do that.

^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Dear Stefan,

Thanks for the review and remedy.
      Certainly, I will again verify the patch once it being committed into
official svn-stream  (Looks not in r1860 yet?)
      and/or 0.9.8-release.

Thank you
Tai-Hong Wu (T.H.Wu Type

2014-12-26 23:36 GMT+08:00 Stefan Tauner 
:
> Silvermont (Bay Trail, Rangeley, Avoton) seems to still use the old
> density encoding with 3 bits per chip. Documentation is unavailable
> (held concealed by Intel) but thanks to the efforts of Tai-Hong
> (Type) Wu the layout is clear now. This patch is based on his one
> but solves the issue differently reducing the code complexity.
>
> Signed-off-by: T.H.Wu Type 
> Signed-off-by: Stefan Tauner
>
> Hello Tai-Hong,
>
> thanks a lot for your patch! And sorry for the late reply. Now that
> I got some time I have looked into your patch. Your solution seems
> to work fine but I decided that I'd rather move the dual output
> supported bit to the "common" bit field struct and rename all
> involved structs accordingly. This gets rid of the rather unreadable
> double ternary branch.
>
> I will probably commit this patch before you get a chance to test it
> again because I want to do the 0.9.8 release soonish, but I'd
> appreciate a later test very much too if you want to do that.
>
> ich_descriptors.c | 38 +++++++++++++++++++-------------------
>  ich_descriptors.h | 18 +++++++-----------
>  2 files changed, 26 insertions(+), 30 deletions(-)
>
> diff --git a/ich_descriptors.c b/ich_descriptors.c
> index 1966f66..90f70ee 100644
> --- a/ich_descriptors.c
> +++ b/ich_descriptors.c
> @@ -127,27 +127,27 @@ static const char *pprint_density(enum ich_chipset
> cs, const struct ich_descript
>         case CHIPSET_ICH10:
>         case CHIPSET_5_SERIES_IBEX_PEAK:
>         case CHIPSET_6_SERIES_COUGAR_POINT:
>   case CHIPSET_7_SERIES_PANTHER_POINT: {
>   case CHIPSET_7_SERIES_PANTHER_POINT:
>   case CHIPSET_BAYTRAIL: {
>           uint8_t size_enc;
>           if (idx == 0) {
>                   size_enc = desc->component.old.comp1_density;
>                   size_enc = desc->component.dens_old.comp1_density;
>           } else {
>                   size_enc = desc->component.old.comp2_density;
>                   size_enc = desc->component.dens_old.comp2_density;
>           }
>           if (size_enc > 5)
>                   return "reserved";
>           return size_str[size_enc];
>   }
>   case CHIPSET_8_SERIES_LYNX_POINT:
>   case CHIPSET_BAYTRAIL:
>   case CHIPSET_8_SERIES_LYNX_POINT_LP:
>   case CHIPSET_8_SERIES_WELLSBURG:
>   case CHIPSET_9_SERIES_WILDCAT_POINT: {
>           uint8_t size_enc;
>           if (idx == 0) {
>                   size_enc = desc->component.new.comp1_density;
>                   size_enc = desc->component.dens_new.comp1_density;
>           } else {
>                   size_enc = desc->component.new.comp2_density;
>                   size_enc = desc->component.dens_new.comp2_density;
>           }
>           if (size_enc > 7)
>                   return "reserved";
>
> @@ -207,16 +207,16 @@ void prettyprint_ich_descriptor_component(enum
> ich_chipset cs, const struct ich_
>                 msg_pdbg2("Component 2 density:            %s\n",
> pprint_density(cs, desc, 1));
>         else
>                 msg_pdbg2("Component 2 is not used.\n");
>   msg_pdbg2("Read Clock Frequency:           %s\n", pprint_freq(cs,
>
> desc->component.common.freq_read));
>   msg_pdbg2("Read ID and Status Clock Freq.: %s\n", pprint_freq(cs,
>
> desc->component.common.freq_read_id));
>   msg_pdbg2("Write and Erase Clock Freq.:    %s\n", pprint_freq(cs,
>
> desc->component.common.freq_write));
>   msg_pdbg2("Fast Read is %ssupported.\n",
>
> desc->component.common.fastread ? "" : "not ");
>   if (desc->component.common.fastread)
>   msg_pdbg2("Read Clock Frequency:           %s\n", pprint_freq(cs,
>
> desc->component.modes.freq_read));
>   msg_pdbg2("Read ID and Status Clock Freq.: %s\n", pprint_freq(cs,
>
> desc->component.modes.freq_read_id));
>   msg_pdbg2("Write and Erase Clock Freq.:    %s\n", pprint_freq(cs,
>
> desc->component.modes.freq_write));
>   msg_pdbg2("Fast Read is %ssupported.\n",
>
> desc->component.modes.fastread ? "" : "not ");
>   if (desc->component.modes.fastread)
>           msg_pdbg2("Fast Read Clock Frequency:      %s\n",
>                     pprint_freq(cs,
>
> desc->component.common.freq_fastread));
>                     pprint_freq(cs,
>
> desc->component.modes.freq_fastread));
>         if (cs > CHIPSET_6_SERIES_COUGAR_POINT)
>                 msg_pdbg2("Dual Output Fast Read Support:  %sabled\n",
>                     desc->component.new.dual_output ? "dis" : "en");
>                     desc->component.modes.dual_output ? "dis" :
>
> "en");
>         if (desc->component.FLILL == 0)
>                 msg_pdbg2("No forbidden opcodes.\n");
>         else {
> @@ -816,22 +816,22 @@ int getFCBA_component_density(enum ich_chipset cs,
> const struct ich_descriptors
>         case CHIPSET_5_SERIES_IBEX_PEAK:
>         case CHIPSET_6_SERIES_COUGAR_POINT:
>         case CHIPSET_7_SERIES_PANTHER_POINT:
>   case CHIPSET_BAYTRAIL:
>           if (idx == 0) {
>                   size_enc = desc->component.old.comp1_density;
>                   size_enc = desc->component.dens_old.comp1_density;
>           } else {
>                   size_enc = desc->component.old.comp2_density;
>                   size_enc = desc->component.dens_old.comp2_density;
>           }
>           size_max = 5;
>           break;
>   case CHIPSET_8_SERIES_LYNX_POINT:
>   case CHIPSET_BAYTRAIL:
>   case CHIPSET_8_SERIES_LYNX_POINT_LP:
>   case CHIPSET_8_SERIES_WELLSBURG:
>   case CHIPSET_9_SERIES_WILDCAT_POINT:
>           if (idx == 0) {
>                   size_enc = desc->component.new.comp1_density;
>                   size_enc = desc->component.dens_new.comp1_density;
>           } else {
>                   size_enc = desc->component.new.comp2_density;
>                   size_enc = desc->component.dens_new.comp2_density;
>           }
>           size_max = 7;
>           break;
>
> @@ -842,7 +842,7 @@ int getFCBA_component_density(enum ich_chipset cs,
> const struct ich_descriptors
>         }
>     if (size_enc > size_max) {
>           msg_perr("Density of ICH SPI component with index %d is
>
> invalid."
>           msg_perr("Density of ICH SPI component with index %d is
>
> invalid.\n"
>                          "Encoded density is 0x%x while maximum allowed is
> 0x%x.\n",
>                          idx, size_enc, size_max);
>                 return -1;
> diff --git a/ich_descriptors.h b/ich_descriptors.h
> index c41f9d9..2c21598 100644
> --- a/ich_descriptors.h
> +++ b/ich_descriptors.h
> @@ -123,23 +123,19 @@ struct ich_desc_component {
>                                  freq_fastread  :3,
>                                  freq_write     :3,
>                                  freq_read_id   :3,
>                                           :2;
>           } common;
>                            dual_output    :1, /* new since Cougar
>
> Point/6 */
>                                           :1;
>           } modes;
>           struct {
>                   uint32_t comp1_density  :3,
>                            comp2_density  :3,
>                                           :11,
>                                           :13,
>                                           :2;
>           } old;
>                                           :26;
>           } dens_old;
>           struct {
>                   uint32_t comp1_density  :4, /* new since Lynx
>
> Point/8 */
>                                  comp2_density  :4,
>                                           :9,
>                                           :13,
>                            dual_output    :1, /* new since Cougar
>
> Point/6 */
>                                           :1;
>           } new;
>                                           :24;
>           } dens_new;
>   };
>   union {                 /* 0x04 */
>           uint32_t FLILL; /* Flash Invalid Instructions Register */