The read is indeed not necessary. Setting the bit in the clock_reg (System Module Stop Control Register) would suffice.
-----Ursprüngliche Nachricht----- Von: Jan Kiszka [mailto:[email protected]] Gesendet: Freitag, 17. November 2017 13:49 An: Ralf Ramsauer; von Wiarda, Jan; JailhouseMailingListe Betreff: Re: [PATCH] arm-common: Force enabling the clock gate of the debug console On 2017-11-17 13:42, Ralf Ramsauer wrote: > > > On 11/17/2017 01:38 PM, Jan Kiszka wrote: >> On 2017-11-17 13:36, Ralf Ramsauer wrote: >>> Hi Jan, >>> >>> On 11/17/2017 11:40 AM, [email protected] wrote: >>>> From: Ruediger Fichter <[email protected]> >>>> >>>> Enable the clock gate of the debug console, even in the case it has been >>>> turned off i.e. because the root cell is not using the UART. >>>> >>>> Signed-off-by: Ruediger Fichter <[email protected]> >>>> Signed-off-by: Jan von Wiarda <[email protected]> >>>> --- >>>> hypervisor/arch/arm-common/uart-hscif.c | 7 +++++++ >>>> 1 file changed, 7 insertions(+) >>>> >>>> diff --git a/hypervisor/arch/arm-common/uart-hscif.c >>>> b/hypervisor/arch/arm-common/uart-hscif.c >>>> index 4eb7534..28eae9a 100644 >>>> --- a/hypervisor/arch/arm-common/uart-hscif.c >>>> +++ b/hypervisor/arch/arm-common/uart-hscif.c >>>> @@ -28,10 +28,17 @@ >>>> >>>> #define HSCIF_FIFO_SIZE 128 >>>> >>>> +#define CON_CLOCK_STS_REG 0xe61501c4 >>> this address looks like it is dependent of the specific board, and >>> not of the UART device, right? In this case, it should not be >>> hardcoded to the UART driver, as this would break other platforms >>> that use the same driver. >> >> Maybe it can be derived from clock_reg, should is defined via the >> system config. > Still, then this will only work for that specific clock controller > that is used in this Renesas device which comes with a certain register > distance. > > I think it's better to stick to the set/clear bit pattern of one > single register for gating clocks. At least if possible. OK, maybe we can clarify first why the value to be written needs to be read from a separate register. I'm afraid we will need something better for clock gating anyway. If there is no clean solution yet, we could also take this with a comment and address it along with a general clock partitioning approach. Jan > > Ralf >> >> Jan >> >>>> + >>>> static void uart_init(struct uart_chip *chip) { >>>> + void *clock_reg = (void*)(unsigned long)chip->debug_console->clock_reg; >>>> + void *clock_sts_reg = (void*)(unsigned long)CON_CLOCK_STS_REG; >>>> + unsigned int gate_nr = chip->debug_console->gate_nr; >>>> u16 hsscr; >>>> >>>> + mmio_write32(clock_reg, mmio_read32(clock_sts_reg) & ~(1 << >>>> +gate_nr)); >>> I don't get this line. You're reading from clock_sts_reg, unset bit >>> gate_nr and write the result to clock_reg. >>> >>> Why can't you read from a clock register, set/clear the >>> uart-specific gate bit and write it back to the same address? That's >>> the pattern we use in all other UART drivers. >>> >>> Ralf >>>> + >>>> if (chip->debug_console->divider) { >>>> hsscr = mmio_read16(chip->virt_base + HSCIF_HSSCR); >>>> mmio_write16(chip->virt_base + HSCIF_HSSCR, >>>> >> >> -- Siemens AG, Corporate Technology, CT RDA ITP SES-DE Corporate Competence Center Embedded Linux -- You received this message because you are subscribed to the Google Groups "Jailhouse" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
<<attachment: winmail.dat>>
