Hi Arnd, Thank you for taking time to answer my questions. On Thu, Jan 12, 2023 at 02:37:53PM +0100, Arnd Bergmann wrote: > On Thu, Jan 12, 2023, at 14:16, Clay Chang wrote: > > On Tue, Jan 10, 2023 at 10:49:36AM +0100, Krzysztof Kozlowski wrote: > >> On 10/01/2023 05:25, clayc@xxxxxxx wrote: > > >> > @@ -0,0 +1,36 @@ > >> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > >> > +%YAML 1.2 > >> > +--- > >> > +$id: http://devicetree.org/schemas/soc/hpe/hpe,gxp-srom.yaml# > >> > +$schema: http://devicetree.org/meta-schemas/core.yaml# > >> > + > >> > +title: HPE GXP SoC SROM Control Register > >> > + > >> > +maintainers: > >> > + - Clay Chang <clayc@xxxxxxx> > >> > + > >> > +description: |+ > >> > + The SROM control register can be used to configure LPC related legacy > >> > + I/O registers. > >> > >> And why this is a hardware? No, you now add fake devices to be able to > >> write some stuff from user-space... Otherwise this needs proper hardware > >> description. > > > > Thank you for commenting on this. You are right, this is not a real > > hardware device, but simply exposes MMIO regions to the user-space. > > Maybe we should rewrite this as a syscon driver. Is writing a syscon > > driver a right direction? > > There are two completely separate questions about the DT binding > and about the user-visible interface. > > The binding needs to properly identify what this device is. I don't > think anyone without the datasheet can tell you the right answer > here, it really depends what the other registers do. If there are > lots of unrelated registers in a small area, a syscon might be > the right answer, but if they are all related to an external > memory bus, then categorizing it as a memory controller may > be more appropriate. Our use-cases are more like some register accesses not related to an external memory bus, so syscon might be a better fit. > > For the user interface side, I don't really like the idea of > having a hardware register directly exposed as driver in > drivers/soc, this generally makes it impossible to have portable > userspace that works across implementations of multiple SoC > vendors, and it makes it too easy to come up with an ad-hoc > interface to make a chip work for a particular use case when > a more general solution would be better. > I agree with you. I have one question though: if we create a 'hpe' directory under drivers/soc, and put all HPE BMC specific drivers there, do you think this proper? > Again, it's hard for me to tell why this even needs to be runtime > configurable, please try to describe what type of application > would access the sysfs interface here, and why this can't just > be set to a fixed value by bootloader or kernel without user > interaction. The register is used for communication and synchronization between the BMC and the host. During runtime, user-space daemons configures the value of the register for interactions. > > Arnd Thanks, Clay