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.
