Re: [PATCH net 2/2] net: dsa: sja1105: always enable the send_meta options

[Simon Horman]

On Fri, Jun 30, 2023 at 07:53:00PM +0300, Vladimir Oltean wrote:
> Hi Simon,
> 
> On Fri, Jun 30, 2023 at 03:35:23PM +0200, Simon Horman wrote:
> > Hi Vladimir,
> > 
> > this patch isn't that big, so I'm ok with it.  But it also isn't that
> > small, so I'd just like to mention that a different approach might be a
> > small patch that enables meta frame generation unconditionally, as a fix.
> > And then, later, some cleanup, which seems to comprise most of this patch.
> > 
> > I do admit that I didn't try this. So it might not be sensible.  And as I
> > said, I am ok with this patch. But I did think it was worth mentioning.
> > 
> > Reviewed-by: Simon Horman <simon.horman@xxxxxxxxxxxx>
> > 
> > ...
> 
> Thanks for the feedback.
> 
> You're saying that it's preferable to leave sja1105_rxtstamp_set_state()
> and sja1105_rxtstamp_get_state() where they are, accessible through
> tagger_data->rxtstamp_set_state() and tagger_data->rxtstamp_get_state(),
> but to only call tagger_data->rxtstamp_set_state() once, statically with
> "true", and to never call tagger_data->rxtstamp_get_state()?

I'm not the ultimate authority on what is and isn't acceptable.
But, yes, I was suggesting something like that.

> I probably could, but I'd have to delay the tagger_data->rxtstamp_set_state()
> call until sja1105_connect_tag_protocol(); it's not going to be available
> earlier. And this is going to be just filler code that I will then delete
> as soon as net-next reopens.

Right, so there is complexity. And dead code.
Or we could just cut to the chase, which is what you have done.

So it seems your approach is best.