On Wed, 8 Jan 2025 09:36:45 -0800 Jakub Kicinski <kuba@xxxxxxxxxx> wrote: > On Wed, 8 Jan 2025 10:27:36 +0100 Kory Maincent wrote: > > > Is there a reason this is defined in ethtool.h? > > > > I moved in to ethtool because the PSE drivers does not need it anymore. > > I can keep it in pse.h. > > > > > I have a weak preference towards keeping it in pse-pd/pse.h > > > since touching ethtool.h rebuilds bulk of networking code. > > > From that perspective it's also suboptimal that pse-pd/pse.h > > > pulls in ethtool.h. > > > > Do you prefer the other way around, ethtool.h pulls in pse.h? > > No, no, I'd say the order of deceasing preference is: > - headers are independent > - smaller header includes bigger one > - bigger one includes smaller one Ok good to know. > > Several structure are used in ethtool, PSE core and even drivers at the same > > time so I don't have much choice. Or, is it preferable to add a new header? > > > > From a quick look it seemed like pse.h definitely needs the enums from > the uAPI. But I couldn't find anything from the kernel side ethtool.h > header it'd actually require (struct ethtool_c33_pse_ext_state_info > can be moved to pse.h as well?). > > Anyways, it's not a major issue for existing code, more of forward guidance. I am fully open to guidance to have proper code. Regards, -- Köry Maincent, Bootlin Embedded Linux and kernel engineering https://bootlin.com