Ok, I can't say I have a strong opinion on this either way, so...

Reviewed-by: Glenn Miles <[email protected]>

Thanks,

Glenn

On Tue, 2026-01-06 at 11:07 -0600, Caleb Schlossin wrote:
> 
> On 1/6/26 10:49 AM, Miles Glenn wrote:
> > Hi Caleb.  I wonder if it makes sense to upstream this commit since I
> > suspect that most upstream users will not have the "unimp" log messages
> > enabled unless they are debugging a problem and in that case, we would
> > be erroneously masking these unimplemented registers from the logged
> > output.
> > 
> > Thanks,
> > 
> > Glenn
> 
> I understand your point. Here are my thoughts:
> - Cleaning up these logs for valid accesses (PowerVM bringup and development) 
> reduces the overall log output and helps find real errors
> - In the future, there may be a customer that wants to run PowerVM with 
> upstream QEMU. The more we upstream, the easier that will be.
> - In the future, we are going to have a number of cases like this where we 
> accept accesses and don't log for every unimp access (to clean up log 
> output). If we choose to keep those patches private and don't upstream them 
> it's going to increase the number of private patches we keep, making future 
> rebasing more difficult.
> - I'd prefer to upstream more patches, and focus on keeping only the patches 
> we need to private (for confidentiality or other reasons). To make future 
> rebasing easier.
> 
> Feel free to contact me offline, if you'd like to discuss this further.
> 
> Thanks,
> Caleb
> 
> > On Thu, 2025-12-18 at 14:03 -0600, Caleb Schlossin wrote:
> > > This commit suppresses the following informational messages
> > > regarding unimplemented pnv_chiptod registers:
> > > 
> > > pnv_chiptod: unimplemented register: Ox0
> > > pnv_chiptod: unimplemented register: Ox1
> > > pnv_chiptod: unimplemented register: Ox2
> > > pnv_chiptod: unimplemented register: Ox3
> > > pnv_chiptod: unimplemented register: Ox4
> > > pnv_chiptod: unimplemented register: Ox5
> > > pnv_chiptod: unimplemented register: Ox13
> > > 
> > > Signed-off-by: Glenn Miles <[email protected]>
> > > Signed-off-by: Caleb Schlossin <[email protected]>
> > > ---
> > >  hw/ppc/pnv_chiptod.c | 18 ++++++++++++++++++
> > >  1 file changed, 18 insertions(+)
> > > 
> > > diff --git a/hw/ppc/pnv_chiptod.c b/hw/ppc/pnv_chiptod.c
> > > index f887a18cde..cd9b8ee348 100644
> > > --- a/hw/ppc/pnv_chiptod.c
> > > +++ b/hw/ppc/pnv_chiptod.c
> > > @@ -145,6 +145,15 @@ static uint64_t pnv_chiptod_xscom_read(void *opaque, 
> > > hwaddr addr,
> > >              val |= PPC_BIT(4);
> > >          }
> > >          break;
> > > +    case TOD_M_PATH_CTRL_REG:
> > > +    case TOD_PRI_PORT_0_CTRL_REG:
> > > +    case TOD_PRI_PORT_1_CTRL_REG:
> > > +    case TOD_SEC_PORT_0_CTRL_REG:
> > > +    case TOD_SEC_PORT_1_CTRL_REG:
> > > +    case TOD_S_PATH_CTRL_REG:
> > > +    case TOD_TX_TTYPE_2_REG:
> > > +        /* unimplemented, but suppressing logging for now */
> > > +        break;
> > >      default:
> > >          qemu_log_mask(LOG_UNIMP, "pnv_chiptod: unimplemented register: 
> > > Ox%"
> > >                        HWADDR_PRIx "\n", addr >> 3);
> > > @@ -420,6 +429,15 @@ static void pnv_chiptod_xscom_write(void *opaque, 
> > > hwaddr addr,
> > >      case TOD_TX_TTYPE_5_REG:
> > >          pctc->broadcast_ttype(chiptod, offset);
> > >          break;
> > > +    case TOD_M_PATH_CTRL_REG:
> > > +    case TOD_PRI_PORT_0_CTRL_REG:
> > > +    case TOD_PRI_PORT_1_CTRL_REG:
> > > +    case TOD_SEC_PORT_0_CTRL_REG:
> > > +    case TOD_SEC_PORT_1_CTRL_REG:
> > > +    case TOD_S_PATH_CTRL_REG:
> > > +    case TOD_TX_TTYPE_2_REG:
> > > +        /* unimplemented, but suppressing logging for now */
> > > +        break;
> > >      default:
> > >          qemu_log_mask(LOG_UNIMP, "pnv_chiptod: unimplemented register: 
> > > Ox%"
> > >                        HWADDR_PRIx "\n", addr >> 3);


Reply via email to