Hi, On Tue, 22 Aug 2023 at 18:47, Guenter Roeck <linux@xxxxxxxxxxxx> wrote: > > On Tue, Aug 22, 2023 at 02:32:31PM +0530, Naresh Solanki wrote: > > Hi > > > > On Fri, 18 Aug 2023 at 14:53, Krzysztof Kozlowski > > <krzysztof.kozlowski@xxxxxxxxxx> wrote: > > > > > > On 16/08/2023 10:51, Naresh Solanki wrote: > > > > Hi Krzysztof, > > > > > > > > On Tue, 15 Aug 2023 at 01:02, Krzysztof Kozlowski > > > > <krzysztof.kozlowski@xxxxxxxxxx> wrote: > > > >> > > > >> On 11/08/2023 18:00, Naresh Solanki wrote: > > > >>> Hi, > > > >>> > > > >>> On Tue, 8 Aug 2023 at 19:58, Conor Dooley <conor@xxxxxxxxxx> wrote: > > > >>>> > > > >>>> On Tue, Aug 08, 2023 at 07:10:08AM -0700, Guenter Roeck wrote: > > > >>>>> On 8/8/23 04:46, Conor Dooley wrote: > > > >>>>>> On Wed, Aug 02, 2023 at 09:31:51PM +0200, Naresh Solanki wrote: > > > >>>>>>> From: Patrick Rudolph <patrick.rudolph@xxxxxxxxxxxxx> > > > >>>>>>> > > > >>>>>>> The TDA38640 chip has different output control mechanisms depending on > > > >>>>>>> its mode of operation. When the chip is in SVID mode, only > > > >>>>>>> hardware-based output control is supported via ENABLE pin. However, when > > > >>>>>>> it operates in PMBus mode, software control works perfectly. > > > >>>>>>> > > > >>>>>>> To enable software control as a workaround in SVID mode, add the DT > > > >>>>>>> property 'infineon,en-svid-control'. This property will enable the > > > >>>>>>> workaround, which utilizes ENABLE pin polarity flipping for output when > > > >>>>>>> the chip is in SVID mode. > > > >>>>>> > > > >>>>>> Why do you need a custom property for this? How come it is not possible > > > >>>>>> to determine what bus you are on? > > > >>>>>> > > > >>>>> > > > >>>>> That is not the point. Yes, it can be detected if the control method is > > > >>>>> PMBus or SVID. However, in SVID mode, SVID is supposed to control the > > > >>>>> output, not PMBUs. This is bypassed by controlling the polarity of the > > > >>>>> (physical) output enable signal. We do _not_ want this enabled automatically > > > >>>>> in SVID mode. Its side effects on random boards using this chip are unknown. > > > >>>>> Thus, this needs a property which specifically enables this functionality > > > >>>>> for users who _really_ need to use it and (hopefully) know what they are > > > >>>>> doing. > > > >>>> > > > >>>> Hmm, reading this it makes a lot more sense why this is a property - I > > > >>>> guess I just struggled to understand the commit message here, > > > >>>> particularly what the benefit of using the workaround is. I'm still > > > >>>> having difficulty parsing the commit & property text though - its > > > >>>> unclear to me when you would need to use it - so I will stay out > > > >>>> of the way & let Rob or Krzysztof handle things. > > > >>> > > > >>> To provide context, my system employs a unique power sequence > > > >>> strategy utilizing a BMC (Baseboard Management Controller), > > > >>> rendering the reliance on the ENABLE pin unnecessary. > > > >>> In this configuration, the ENABLE pin is grounded in the hardware. > > > >>> While most regulators facilitate PMBus Operation for output control, > > > >>> the TDA38640 chip, when in SVID mode, is constrained by the > > > >>> ENABLE pin to align with Intel specifications. > > > >>> My communication with Infineon confirmed that the recommended > > > >>> approach is to invert the Enable Pin for my use case. > > > >>> > > > >>> Since this is not typically the use case for most setup & hence DT property > > > >>> is must for enabling the special case. > > > >>> > > > >>> For further insight into my setup's power sequence strategy, you can > > > >>> refer to the following link: https://github.com/9elements/pwrseqd > > > >>> > > > >> > > > >> This justifies to me the property, but still you described desired > > > >> driver behavior, not the hardware characteristic. Don't describe what > > > >> you want to control, but describe the entire system. > > > > I guess by entire system you mean how the regulators(including > > > > TDA38640) connected & operated in our setup ? > > > > > > I mean, property name and description should say what is the > > > characteristic of the hardware/firmware/entire system. > > Based on your feedback, will update to below: > > infineon,fixed-level-en-pin: > > description: | > > Indicate the ENABLE pin is set at fixed level or left > > unconnected(has internal pull-up). > > type: boolean > > Messy, because while it reflects physical connectivity, it doesn't reflect > its use in the system at all. The pin may be at fixed level or left > unconnected, but the system vendor doesn't want to give users the > means to control output power (which would be the normal situation). Maybe this would be better ? infineon,svid-mode-fixed-en-pin description: | Indicate the ENABLE pin is set at fixed level or left unconnected(has internal pull-up) which chip in SVID mode. Regards, Naresh > > But then, if that is the only way to get a property accepted, so be it. > > Guenter