Re: BUG: iio: mpu3050: Wrong temperature scale
19.04.2021 13:07, Linus Walleij пишет: > On Mon, Apr 19, 2021 at 8:06 AM Dmitry Osipenko wrote: > >> The driver uses >> (x+23000)/280 formula for the conversion of raw temperature value, which >> gives 82C for x=0, thus apparently formula is wrong because x=5 >> should give us ~25C. >> >> I tried to search for the datasheet with the formula, but couldn't find it. > > There is no public datasheet. I have never seen a non-public datasheet > either. > > As the initial submission of the driver says: > > "This driver is based on information from the rough input driver > in drivers/input/misc/mpu3050.c and the scratch misc driver > posted by Nathan Royer in 2011. Some years have passed but this > is finally a fully-fledged driver for this gyroscope. It was > developed and tested on the Qualcomm APQ8060 Dragonboard." > > Nathans submission: > https://lore.kernel.org/lkml/1309486707-1658-1-git-send-email-nro...@invensense.com/ > (you find the threads at the bottom) > > This submission came from inside Invensense so it is the closest > authoritative source we have. > >> Linus, will you be able to check whether the formula used by the driver >> is correct? Thanks in advance. > > Sadly the code is the documentation when it comes to Invensense stuff, > I am CC:ing Nathans Invensense address in the vain hope he is still > working there and could help, also CC to Jean-Baptiste who was > there last year and maybe can help out. > > I don't anymore remember exactly how I found this equation, > but it wasn't from any datasheet. I vaguely remember browsing > through some Android userspace sensor code. > > What I tend to do is dig around in old mobile > phone Android trees, and there you sometimes find this information > in different GPL code drops. I bet I got it from browsing some of > those. > > Here is an example (Tegra): > https://android.googlesource.com/kernel/tegra/+/dba2740d025c8e7e7e3c61d84a4f964d2c1c0ac9/drivers/misc/inv_mpu > > Worst case what one *can* do is to calibrate the scale, like put > the device in a controlled environment of some two reasonably > far apart temperatures and measure, assuming it is at least > linear. Some professionals use controlled environment > chambers for this. But I hope there is a better way. Linus, thank you very much for the answer! I found a non-kernel example which uses a similar equation [1], but in a different form. The main difference is that the Arduino code interprets a raw temperature value as a signed integer, while upstream assumes it's unsigned. [1] https://github.com/blaisejarrett/Arduino-Lib.MPU3050/blob/master/MPU3050lib.cpp#L111 Still, even if assume that the raw temperature is a signed s16 value, it gives us ~35C in a result, which should be off by ~10C. Certainly a manual calibration is an option, but we will try to wait for the answer from Nathans and Jean-Baptiste before going that route.
BUG: iio: mpu3050: Wrong temperature scale
Hello, Svyatoslav and me found that the MPU3050 IIO driver reports temperature that is x10 larger than it should be on Asus Transformer TF201 and Acer A500 tablet devices running mainline kernel. The driver uses (x+23000)/280 formula for the conversion of raw temperature value, which gives 82C for x=0, thus apparently formula is wrong because x=5 should give us ~25C. I tried to search for the datasheet with the formula, but couldn't find it. Linus, will you be able to check whether the formula used by the driver is correct? Thanks in advance.
Re: [PATCH v1 1/2] iommu/tegra-smmu: Defer attachment of display clients
08.04.2021 17:07, Dmitry Osipenko пишет: >> Whatever happened to the idea of creating identity mappings based on the >> obscure tegra_fb_mem (or whatever it was called) command-line option? Is >> that command-line not universally passed to the kernel from bootloaders >> that initialize display? > This is still a good idea! The command-line isn't universally passed > just because it's up to a user to override the cmdline and then we get a > hang (a very slow boot in reality) on T30 since display client takes out > the whole memory bus with the constant SMMU faults. For example I don't > have that cmdline option in my current setups. > Thinking a bit more about the identity.. have you consulted with the memory h/w people about whether it's always safe to enable ASID in a middle of DMA request?
Re: [PATCH v2] ata: ahci_tegra: call tegra_powergate_power_off only when PM domain is not present
08.04.2021 23:55, Sowjanya Komatineni пишет: > This patch adds check to call legacy power domain API > tegra_powergate_power_off() only when PM domain is not present. > > This is a follow-up patch to Tegra186 AHCI support patch series. > --- > drivers/ata/ahci_tegra.c | 6 -- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/ata/ahci_tegra.c b/drivers/ata/ahci_tegra.c > index 56612af..4fb94db 100644 > --- a/drivers/ata/ahci_tegra.c > +++ b/drivers/ata/ahci_tegra.c > @@ -268,7 +268,8 @@ static int tegra_ahci_power_on(struct ahci_host_priv > *hpriv) > disable_power: > clk_disable_unprepare(tegra->sata_clk); > > - tegra_powergate_power_off(TEGRA_POWERGATE_SATA); > + if (!tegra->pdev->dev.pm_domain) > + tegra_powergate_power_off(TEGRA_POWERGATE_SATA); > > disable_regulators: > regulator_bulk_disable(tegra->soc->num_supplies, tegra->supplies); > @@ -287,7 +288,8 @@ static void tegra_ahci_power_off(struct ahci_host_priv > *hpriv) > reset_control_assert(tegra->sata_cold_rst); > > clk_disable_unprepare(tegra->sata_clk); > - tegra_powergate_power_off(TEGRA_POWERGATE_SATA); > + if (!tegra->pdev->dev.pm_domain) > + tegra_powergate_power_off(TEGRA_POWERGATE_SATA); > > regulator_bulk_disable(tegra->soc->num_supplies, tegra->supplies); > } > Looks good, thank you. Reviewed-by: Dmitry Osipenko
Re: [PATCH v3 1/1] dt-bindings: memory: tegra20: emc: Convert to schema
08.04.2021 23:29, Rob Herring пишет:
> On Sun, Apr 04, 2021 at 06:55:01PM +0300, Dmitry Osipenko wrote:
>> Convert Tegra20 External Memory Controller binding to schema.
>>
>> Signed-off-by: Dmitry Osipenko
>> ---
>> .../memory-controllers/nvidia,tegra20-emc.txt | 130
>> .../nvidia,tegra20-emc.yaml | 303 ++
>> 2 files changed, 303 insertions(+), 130 deletions(-)
>> delete mode 100644
>> Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.txt
>> create mode 100644
>> Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.yaml
>>
>> diff --git
>> a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.txt
>>
>> b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.txt
>> deleted file mode 100644
>> index d2250498c36d..
>> ---
>> a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.txt
>> +++ /dev/null
>> @@ -1,130 +0,0 @@
>> -Embedded Memory Controller
>> -
>> -Properties:
>> -- name : Should be emc
>> -- #address-cells : Should be 1
>> -- #size-cells : Should be 0
>> -- compatible : Should contain "nvidia,tegra20-emc".
>> -- reg : Offset and length of the register set for the device
>> -- nvidia,use-ram-code : If present, the sub-nodes will be addressed
>> - and chosen using the ramcode board selector. If omitted, only one
>> - set of tables can be present and said tables will be used
>> - irrespective of ram-code configuration.
>> -- interrupts : Should contain EMC General interrupt.
>> -- clocks : Should contain EMC clock.
>> -- nvidia,memory-controller : Phandle of the Memory Controller node.
>> -- #interconnect-cells : Should be 0.
>> -- operating-points-v2: See ../bindings/opp/opp.txt for details.
>> -
>> -For each opp entry in 'operating-points-v2' table:
>> -- opp-supported-hw: One bitfield indicating SoC process ID mask
>> -
>> -A bitwise AND is performed against this value and if any bit
>> -matches, the OPP gets enabled.
>> -
>> -Optional properties:
>> -- power-domains: Phandle of the SoC "core" power domain.
>> -
>> -Child device nodes describe the memory settings for different
>> configurations and clock rates.
>> -
>> -Example:
>> -
>> -opp_table: opp-table {
>> -compatible = "operating-points-v2";
>> -
>> -opp@3600 {
>> -opp-microvolt = <95 95 130>;
>> -opp-hz = /bits/ 64 <3600>;
>> -};
>> -...
>> -};
>> -
>> -memory-controller@7000f400 {
>> -#address-cells = < 1 >;
>> -#size-cells = < 0 >;
>> -#interconnect-cells = <0>;
>> -compatible = "nvidia,tegra20-emc";
>> -reg = <0x7000f400 0x400>;
>> -interrupts = <0 78 0x04>;
>> -clocks = <_car TEGRA20_CLK_EMC>;
>> -nvidia,memory-controller = <>;
>> -power-domains = <>;
>> -operating-points-v2 = <_table>;
>> -}
>> -
>> -
>> -Embedded Memory Controller ram-code table
>> -
>> -If the emc node has the nvidia,use-ram-code property present, then the
>> -next level of nodes below the emc table are used to specify which settings
>> -apply for which ram-code settings.
>> -
>> -If the emc node lacks the nvidia,use-ram-code property, this level is
>> omitted
>> -and the tables are stored directly under the emc node (see below).
>> -
>> -Properties:
>> -
>> -- name : Should be emc-tables
>> -- nvidia,ram-code : the binary representation of the ram-code board
>> strappings
>> - for which this node (and children) are valid.
>> -
>> -
>> -
>> -Embedded Memory Controller configuration table
>> -
>> -This is a table containing the EMC register settings for the various
>> -operating speeds of the memory controller. They are always located as
>> -subnodes of the emc controller node.
>> -
>> -There are two ways of specifying which tables to use:
>> -
>> -* The simplest is if there is just one set of tables in the device tree,
>> - and they will always be used (based on which frequency is used).
>> - This is the preferred method, especially when firmware can fill in
>> - this information based on the spe
Re: [PATCH v1] ata: ahci_tegra: call tegra_powergate_power_off only when PM domain is not present
08.04.2021 19:40, Sowjanya Komatineni пишет: > This patch adds a check on present of PM domain and calls legacy power > domain API tegra_powergate_power_off() only when PM domain is not present. > > This is a follow-up patch to Tegra186 AHCI support patch series > https://lore.kernel.org/patchwork/cover/1408752/ > > Signed-off-by: Sowjanya Komatineni > > --- > drivers/ata/ahci_tegra.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/ata/ahci_tegra.c b/drivers/ata/ahci_tegra.c > index 56612af..bd484dd 100644 > --- a/drivers/ata/ahci_tegra.c > +++ b/drivers/ata/ahci_tegra.c > @@ -287,7 +287,8 @@ static void tegra_ahci_power_off(struct ahci_host_priv > *hpriv) > reset_control_assert(tegra->sata_cold_rst); > > clk_disable_unprepare(tegra->sata_clk); > - tegra_powergate_power_off(TEGRA_POWERGATE_SATA); > + if (!tegra->pdev->dev.pm_domain) > + tegra_powergate_power_off(TEGRA_POWERGATE_SATA); > > regulator_bulk_disable(tegra->soc->num_supplies, tegra->supplies); > } > There are two instances of tegra_powergate_power_off() in the driver.
Re: [PATCH v4 3/3] ata: ahci_tegra: Add AHCI support for Tegra186
08.04.2021 16:06, Thierry Reding пишет:
> On Thu, Apr 08, 2021 at 02:25:19AM +0300, Dmitry Osipenko wrote:
>> 08.04.2021 02:00, Sowjanya Komatineni пишет:
>>>
>>> On 4/7/21 3:57 PM, Sowjanya Komatineni wrote:
>>>>
>>>> On 4/7/21 2:36 PM, Dmitry Osipenko wrote:
>>>>> 07.04.2021 04:25, Sowjanya Komatineni пишет:
>>>>>> + if (!tegra->pdev->dev.pm_domain) {
>>>>>> + ret = tegra_powergate_sequence_power_up(TEGRA_POWERGATE_SATA,
>>>>>> + tegra->sata_clk,
>>>>>> + tegra->sata_rst);
>>>>>> + if (ret)
>>>>>> + goto disable_regulators;
>>>>>> + }
>>>>> Hi,
>>>>>
>>>>> Why you haven't added condition for tegra_powergate_power_off()? I think
>>>>> it should break GENPD and legacy PD API isn't not supported by T186
>>>>> at all.
>>>>>
>>>>> I'm also not sure whether the power up/down sequence is correct using
>>>>> GENPD.
>>>>>
>>>>> Moreover the driver doesn't support runtime PM, so GENPD should be
>>>>> always off?
>>>>
>>>> This driver already using legacy PD API's so thought its supported and
>>>> added power domain device check during powergate_sequence_power_up and
>>>> yes same should apply for powergate_power_off as well. But if legacy
>>>> PD is not supported by T186 then not sure why original driver even
>>>> using these API's.
>>>>
>>>>
>>> Sorry just took a look and driver supports T210 and prior tegra as well.
>>> T210 and prior supports legacy PD and this check is applicable for
>>> those. So we should add power domain device check for power off as well.
>>
>> You could fix it with a follow up patch. Please try to test that
>> power-off works properly, at least try to unload the driver module and
>> re-load it.
>
> Agreed, this should have the same check as above for
> tegra_powergate_power_off(). It currently works fine because on Tegra186
> tegra_powergate_power_off() (and all the other legacy APIs for that
> matter) will abort early since no power gates are implemented. The AHCI
> driver doesn't check for errors, so this will just fail silently. It's
> better to be symmetric, though, and add the check in both paths.
I missed that tegra_powergate_power_off() usage isn't fatal if GENPD is
used, thank you for the clarification.
>>> But for T186, we should have GENPD working once we add runtime PM
>>> support to driver.
>>>
>>> Preetham/Thierry, Can you confirm where SATA is un powergated prior to
>>> kernel?
>>>
>>>
>>>> But as RPM is not implemented yet for this driver, GENPD will be OFF
>>>> but SATA is not in power-gate by the time kernel starts and
>>>> functionally works.
>>>>
>>>> But with RPM implementation, I guess we can do proper power gate on/off.
>>>>
>>
>> I now recalled that GENPD turns ON all domains by default and then turns
>> them OFF only when driver entered into the RPM-suspended state. This
>> means that AHCI GENPD should be always-ON for T186, which should be okay
>> if this doesn't break power sequences.
>
> Yeah, the generic PM domain will just stay enabled after probe and until
> remove. This does not impact the power sequences because they have to be
> completely implemented in the power domains code anyway. With the legacy
> API we used to need more rigorous sequences in the individual drivers,
> but with generic PM domains none of that should be necessary, though it
> also doesn't hurt, so some of the unnecessary clock enablement code is
> kept for simplicity.
>
> To be honest, I'm not sure if it's worth adding runtime PM support for
> this driver. If this top-level layer has a way of getting notification
> when no device was detected, then it might make some sense to turn off
> the power domain and the regulators again, but I'm not sure if that's
> the case. tegra_ahci_host_stop() seems like it might be usable for that
> so yeah, that might work. We currently do turn off the powergate in that
> case, so extending that power optimization to Tegra186 using runtime PM
> makes sense.
Alright, then this all should be good as-is.
Re: [PATCH v1 1/2] iommu/tegra-smmu: Defer attachment of display clients
08.04.2021 16:26, Thierry Reding пишет: > On Thu, Apr 08, 2021 at 02:42:42AM -0700, Nicolin Chen wrote: >> On Mon, Mar 29, 2021 at 02:32:55AM +0300, Dmitry Osipenko wrote: >>> All consumer-grade Android and Chromebook devices show a splash screen >>> on boot and then display is left enabled when kernel is booted. This >>> behaviour is unacceptable in a case of implicit IOMMU domains to which >>> devices are attached during kernel boot since devices, like display >>> controller, may perform DMA at that time. We can work around this problem >>> by deferring the enable of SMMU translation for a specific devices, >>> like a display controller, until the first IOMMU mapping is created, >>> which works good enough in practice because by that time h/w is already >>> stopped. >>> >>> Signed-off-by: Dmitry Osipenko >> >> For both patches: >> Acked-by: Nicolin Chen >> Tested-by: Nicolin Chen >> >> The WAR looks good to me. Perhaps Thierry would give some input. Nicolin, thank you very much for the help! >> Another topic: >> I think this may help work around the mc-errors, which we have >> been facing on Tegra210 also when we enable IOMMU_DOMAIN_DMA. >> (attached a test patch rebasing on these two) > > Ugh... that's exactly what I was afraid of. Now everybody is going to > think that we can just work around this issue with driver-specific SMMU > hacks... > >> However, GPU would also report errors using DMA domain: >> >> nouveau 5700.gpu: acr: firmware unavailable >> nouveau 5700.gpu: pmu: firmware unavailable >> nouveau 5700.gpu: gr: firmware unavailable >> tegra-mc 70019000.memory-controller: gpusrd: read @0xfffbe200: >> Security violation (TrustZone violation) >> nouveau 5700.gpu: DRM: failed to create kernel channel, -22 >> tegra-mc 70019000.memory-controller: gpusrd: read @0xfffad000: >> Security violation (TrustZone violation) >> nouveau 5700.gpu: fifo: SCHED_ERROR 20 [] >> nouveau 5700.gpu: fifo: SCHED_ERROR 20 [] >> >> Looking at the address, seems that GPU allocated memory in 32-bit >> physical address space behind SMMU, so a violation happened after >> turning on DMA domain I guess... > > The problem with GPU is... extra complicated. You're getting these > faults because you're enabling the IOMMU-backed DMA API, which then > causes the Nouveau driver allocate buffers using the DMA API instead of > explicitly allocating pages and then mapping them using the IOMMU API. > However, there are additional patches needed to teach Nouveau about how > to deal with SMMU and those haven't been merged yet. I've got prototypes > of this, but before the whole framebuffer carveout passing work makes > progress there's little sense in moving individual pieces forward. > > One more not to try and cut corners. We know what the right solution is, > even if it takes a lot of work. I'm willing to ack this patch, or some > version of it, but only as a way of working around things we have no > realistic chance of fixing properly anymore. I still think it would be > best if we could derive identity mappings from command-line arguments on > these platforms because I think most of them will actually set that, and > then the solution becomes at least uniform at the SMMU level. > > For Tegra210 I've already laid out a path to a solution that's going to > be generic and extend to Tegra186 and later as well. We still have issues in the DRM and other drivers that don't allow us to flip ON the IOMMU_DOMAIN_DMA support. My patch addresses the issue with the ARM_DMA_USE_IOMMU option, which allocates the unmanaged domain for DMA purposes on ARM32, causing the trouble in the multiplatform kernel configuration since it's not possible to opt-out from ARM_DMA_USE_IOMMU in this case. Perhaps this needs to be clarified in the commit message. https://elixir.bootlin.com/linux/v5.12-rc6/source/arch/arm/mm/dma-mapping.c#L2078 https://elixir.bootlin.com/linux/v5.12-rc6/source/drivers/iommu/iommu.c#L1929
Re: [PATCH v1 1/2] iommu/tegra-smmu: Defer attachment of display clients
08.04.2021 15:40, Thierry Reding пишет:
> On Mon, Mar 29, 2021 at 02:32:55AM +0300, Dmitry Osipenko wrote:
>> All consumer-grade Android and Chromebook devices show a splash screen
>> on boot and then display is left enabled when kernel is booted. This
>> behaviour is unacceptable in a case of implicit IOMMU domains to which
>> devices are attached during kernel boot since devices, like display
>> controller, may perform DMA at that time. We can work around this problem
>> by deferring the enable of SMMU translation for a specific devices,
>> like a display controller, until the first IOMMU mapping is created,
>> which works good enough in practice because by that time h/w is already
>> stopped.
>>
>> Signed-off-by: Dmitry Osipenko
>> ---
>> drivers/iommu/tegra-smmu.c | 71 ++
>> 1 file changed, 71 insertions(+)
>
> In general I do see why we would want to enable this. However, I think
> this is a bad idea because it's going to proliferate the bad practice of
> not describing things properly in device tree.
>
> Whatever happened to the idea of creating identity mappings based on the
> obscure tegra_fb_mem (or whatever it was called) command-line option? Is
> that command-line not universally passed to the kernel from bootloaders
> that initialize display?
This is still a good idea! The command-line isn't universally passed
just because it's up to a user to override the cmdline and then we get a
hang (a very slow boot in reality) on T30 since display client takes out
the whole memory bus with the constant SMMU faults. For example I don't
have that cmdline option in my current setups.
> That idealistic objection aside, this seems a bit over-engineered for
> the hack that it is. See below.
>
>> diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
>> index 602aab98c079..af1e4b5adb27 100644
>> --- a/drivers/iommu/tegra-smmu.c
>> +++ b/drivers/iommu/tegra-smmu.c
>> @@ -60,6 +60,8 @@ struct tegra_smmu_as {
>> dma_addr_t pd_dma;
>> unsigned id;
>> u32 attr;
>> +bool display_attached[2];
>> +bool attached_devices_need_sync;
>> };
>>
>> static struct tegra_smmu_as *to_smmu_as(struct iommu_domain *dom)
>> @@ -78,6 +80,10 @@ static inline u32 smmu_readl(struct tegra_smmu *smmu,
>> unsigned long offset)
>> return readl(smmu->regs + offset);
>> }
>>
>> +/* all Tegra SoCs use the same group IDs for displays */
>> +#define SMMU_SWGROUP_DC 1
>> +#define SMMU_SWGROUP_DCB2
>> +
>> #define SMMU_CONFIG 0x010
>> #define SMMU_CONFIG_ENABLE (1 << 0)
>>
>> @@ -253,6 +259,20 @@ static inline void smmu_flush(struct tegra_smmu *smmu)
>> smmu_readl(smmu, SMMU_PTB_ASID);
>> }
>>
>> +static int smmu_swgroup_to_display_id(unsigned int swgroup)
>> +{
>> +switch (swgroup) {
>> +case SMMU_SWGROUP_DC:
>> +return 0;
>> +
>> +case SMMU_SWGROUP_DCB:
>> +return 1;
>> +
>> +default:
>> +return -1;
>> +}
>> +}
>> +
>
> Why do we need to have this two-level mapping? Do we even need to care
> about the specific swgroups IDs?
It's not clear to me what you're meaning here, the swgroup IDs are used
here for determining whether client belongs to a display controller.
> Can we not just simply check at attach
> time if the client that's being attached is a display client and then
> set atteched_devices_need_sync = true?
The reason I made atteched_devices_need_sync opt-out for display clients
instead of
opt-in is to make it clear and easy to override this option once we will
support the identity mappings.
- attached_devices_need_sync = true;
+ attached_devices_need_sync = no_identity_mapping_for_display;
Re: [PATCH v4 3/3] ata: ahci_tegra: Add AHCI support for Tegra186
08.04.2021 02:00, Sowjanya Komatineni пишет:
>
> On 4/7/21 3:57 PM, Sowjanya Komatineni wrote:
>>
>> On 4/7/21 2:36 PM, Dmitry Osipenko wrote:
>>> 07.04.2021 04:25, Sowjanya Komatineni пишет:
>>>> + if (!tegra->pdev->dev.pm_domain) {
>>>> + ret = tegra_powergate_sequence_power_up(TEGRA_POWERGATE_SATA,
>>>> + tegra->sata_clk,
>>>> + tegra->sata_rst);
>>>> + if (ret)
>>>> + goto disable_regulators;
>>>> + }
>>> Hi,
>>>
>>> Why you haven't added condition for tegra_powergate_power_off()? I think
>>> it should break GENPD and legacy PD API isn't not supported by T186
>>> at all.
>>>
>>> I'm also not sure whether the power up/down sequence is correct using
>>> GENPD.
>>>
>>> Moreover the driver doesn't support runtime PM, so GENPD should be
>>> always off?
>>
>> This driver already using legacy PD API's so thought its supported and
>> added power domain device check during powergate_sequence_power_up and
>> yes same should apply for powergate_power_off as well. But if legacy
>> PD is not supported by T186 then not sure why original driver even
>> using these API's.
>>
>>
> Sorry just took a look and driver supports T210 and prior tegra as well.
> T210 and prior supports legacy PD and this check is applicable for
> those. So we should add power domain device check for power off as well.
You could fix it with a follow up patch. Please try to test that
power-off works properly, at least try to unload the driver module and
re-load it.
> But for T186, we should have GENPD working once we add runtime PM
> support to driver.
>
> Preetham/Thierry, Can you confirm where SATA is un powergated prior to
> kernel?
>
>
>> But as RPM is not implemented yet for this driver, GENPD will be OFF
>> but SATA is not in power-gate by the time kernel starts and
>> functionally works.
>>
>> But with RPM implementation, I guess we can do proper power gate on/off.
>>
I now recalled that GENPD turns ON all domains by default and then turns
them OFF only when driver entered into the RPM-suspended state. This
means that AHCI GENPD should be always-ON for T186, which should be okay
if this doesn't break power sequences.
Re: [PATCH v4 3/3] ata: ahci_tegra: Add AHCI support for Tegra186
07.04.2021 04:25, Sowjanya Komatineni пишет:
> + if (!tegra->pdev->dev.pm_domain) {
> + ret = tegra_powergate_sequence_power_up(TEGRA_POWERGATE_SATA,
> + tegra->sata_clk,
> + tegra->sata_rst);
> + if (ret)
> + goto disable_regulators;
> + }
>
Hi,
Why you haven't added condition for tegra_powergate_power_off()? I think
it should break GENPD and legacy PD API isn't not supported by T186 at all.
I'm also not sure whether the power up/down sequence is correct using GENPD.
Moreover the driver doesn't support runtime PM, so GENPD should be
always off?
[PATCH v3 1/1] dt-bindings: memory: tegra20: emc: Convert to schema
Convert Tegra20 External Memory Controller binding to schema.
Signed-off-by: Dmitry Osipenko
---
.../memory-controllers/nvidia,tegra20-emc.txt | 130
.../nvidia,tegra20-emc.yaml | 303 ++
2 files changed, 303 insertions(+), 130 deletions(-)
delete mode 100644
Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.txt
create mode 100644
Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.yaml
diff --git
a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.txt
b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.txt
deleted file mode 100644
index d2250498c36d..
---
a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.txt
+++ /dev/null
@@ -1,130 +0,0 @@
-Embedded Memory Controller
-
-Properties:
-- name : Should be emc
-- #address-cells : Should be 1
-- #size-cells : Should be 0
-- compatible : Should contain "nvidia,tegra20-emc".
-- reg : Offset and length of the register set for the device
-- nvidia,use-ram-code : If present, the sub-nodes will be addressed
- and chosen using the ramcode board selector. If omitted, only one
- set of tables can be present and said tables will be used
- irrespective of ram-code configuration.
-- interrupts : Should contain EMC General interrupt.
-- clocks : Should contain EMC clock.
-- nvidia,memory-controller : Phandle of the Memory Controller node.
-- #interconnect-cells : Should be 0.
-- operating-points-v2: See ../bindings/opp/opp.txt for details.
-
-For each opp entry in 'operating-points-v2' table:
-- opp-supported-hw: One bitfield indicating SoC process ID mask
-
- A bitwise AND is performed against this value and if any bit
- matches, the OPP gets enabled.
-
-Optional properties:
-- power-domains: Phandle of the SoC "core" power domain.
-
-Child device nodes describe the memory settings for different configurations
and clock rates.
-
-Example:
-
- opp_table: opp-table {
- compatible = "operating-points-v2";
-
- opp@3600 {
- opp-microvolt = <95 95 130>;
- opp-hz = /bits/ 64 <3600>;
- };
- ...
- };
-
- memory-controller@7000f400 {
- #address-cells = < 1 >;
- #size-cells = < 0 >;
- #interconnect-cells = <0>;
- compatible = "nvidia,tegra20-emc";
- reg = <0x7000f400 0x400>;
- interrupts = <0 78 0x04>;
- clocks = <_car TEGRA20_CLK_EMC>;
- nvidia,memory-controller = <>;
- power-domains = <>;
- operating-points-v2 = <_table>;
- }
-
-
-Embedded Memory Controller ram-code table
-
-If the emc node has the nvidia,use-ram-code property present, then the
-next level of nodes below the emc table are used to specify which settings
-apply for which ram-code settings.
-
-If the emc node lacks the nvidia,use-ram-code property, this level is omitted
-and the tables are stored directly under the emc node (see below).
-
-Properties:
-
-- name : Should be emc-tables
-- nvidia,ram-code : the binary representation of the ram-code board strappings
- for which this node (and children) are valid.
-
-
-
-Embedded Memory Controller configuration table
-
-This is a table containing the EMC register settings for the various
-operating speeds of the memory controller. They are always located as
-subnodes of the emc controller node.
-
-There are two ways of specifying which tables to use:
-
-* The simplest is if there is just one set of tables in the device tree,
- and they will always be used (based on which frequency is used).
- This is the preferred method, especially when firmware can fill in
- this information based on the specific system information and just
- pass it on to the kernel.
-
-* The slightly more complex one is when more than one memory configuration
- might exist on the system. The Tegra20 platform handles this during
- early boot by selecting one out of possible 4 memory settings based
- on a 2-pin "ram code" bootstrap setting on the board. The values of
- these strappings can be read through a register in the SoC, and thus
- used to select which tables to use.
-
-Properties:
-- name : Should be emc-table
-- compatible : Should contain "nvidia,tegra20-emc-table".
-- reg : either an opaque enumerator to tell different tables apart, or
- the valid frequency for which the table should be used (in kHz).
-- clock-frequency : the clock frequency for the EMC at which this
- table should be used (in kHz).
-- nvidia,emc-registers : a 46 word array of EMC registers to be programmed
- for operation at the 'clock-frequency' setting.
- The order and contents of the registers are:
-RC, RFC,
[PATCH v3 0/1] NVIDIA Tegra memory improvements
Hi, Here is the last patch of the series which had minor problem in v2, the rest of the patches are already applied by Krzysztof Kozlowski. Changelog: v3: - Added new optional reg property for emc-tables nodes in order to fix dt_binding_check warning. Please note that I will prepare a separate patch for v5.14 that will add the new property to the device-trees since Thierry already sent out PR for v5.13. v2: - Fixed typos in the converted schemas. - Corrected reg entry of tegra20-mc-gart schema to use fixed number of items. - Made power-domain to use maxItems instead of $ref phandle in schemas. Dmitry Osipenko (1): dt-bindings: memory: tegra20: emc: Convert to schema .../memory-controllers/nvidia,tegra20-emc.txt | 130 .../nvidia,tegra20-emc.yaml | 303 ++ 2 files changed, 303 insertions(+), 130 deletions(-) delete mode 100644 Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.txt create mode 100644 Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.yaml -- 2.30.2
Re: [PATCH v2 5/6] dt-bindings: memory: tegra20: emc: Convert to schema
02.04.2021 17:45, Dmitry Osipenko пишет: > 01.04.2021 18:55, Rob Herring пишет: >> On Wed, Mar 31, 2021 at 05:59:39PM +0300, Dmitry Osipenko wrote: >>> 31.03.2021 16:40, Rob Herring пишет: >>>> On Wed, 31 Mar 2021 02:04:44 +0300, Dmitry Osipenko wrote: >>>>> Convert Tegra20 External Memory Controller binding to schema. >>>>> >>>>> Signed-off-by: Dmitry Osipenko >>>>> --- >>>>> .../memory-controllers/nvidia,tegra20-emc.txt | 130 >>>>> .../nvidia,tegra20-emc.yaml | 294 ++ >>>>> 2 files changed, 294 insertions(+), 130 deletions(-) >>>>> delete mode 100644 >>>>> Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.txt >>>>> create mode 100644 >>>>> Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.yaml >>>>> >>>> >>>> My bot found errors running 'make dt_binding_check' on your patch: >>>> >>>> yamllint warnings/errors: >>>> >>>> dtschema/dtc warnings/errors: >>>> Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.example.dts:33.26-55.15: >>>> Warning (unit_address_vs_reg): >>>> /example-0/external-memory-controller@7000f400/emc-tables@0: node has a >>>> unit name, but no reg or ranges property >>>> >>>> See https://patchwork.ozlabs.org/patch/1460288 >>>> >>>> This check can fail if there are any dependencies. The base for a patch >>>> series is generally the most recent rc1. >>>> >>>> If you already ran 'make dt_binding_check' and didn't see the above >>>> error(s), then make sure 'yamllint' is installed and dt-schema is up to >>>> date: >>>> >>>> pip3 install dtschema --upgrade >>>> >>>> Please check and re-submit. >>>> >>> >>> FYI, I'm aware about this warning and I think it's fine to skip it in >>> the case of this binding. >> >> It's not because dt_binding_check should be warning free. >> >>> The dtbs_check doesn't bother with that warning. >> >> With W=1 it will. It's off by default because there are too many of >> these warnings. Patches welcome. > > Such warning could be silenced with some kind of a new pragma option in > schema which will tell what warnings are inappropriate. > > But since there is no such option today, perhaps indeed should be better > to just add a dummy reg property and fix the device-trees. > This actually was my bad, the reg property is already specified in this version of binding as required. Apparently I got confused while was looking at some device-tree that doesn't have the reg property and it should be corrected.
Re: [PATCH v2 0/6] NVIDIA Tegra memory improvements
01.04.2021 20:54, Krzysztof Kozlowski пишет: > On 31/03/2021 01:04, Dmitry Osipenko wrote: >> Hi, >> >> This series replaces the raw voltage regulator with a power domain that >> will be managing SoC core voltage. The core power domain patches are still >> under review, but it's clear at this point that this is the way we will >> implement the DVFS support. >> >> The remaining Tegra20 memory bindings are converted to schema. I also >> made a small improvement to the memory drivers. >> >> Changelog: >> >> v2: - Fixed typos in the converted schemas. >> - Corrected reg entry of tegra20-mc-gart schema to use fixed number of >> items. >> - Made power-domain to use maxItems instead of $ref phandle in schemas. >> >> Dmitry Osipenko (6): >> dt-bindings: memory: tegra20: emc: Replace core regulator with power >> domain >> dt-bindings: memory: tegra30: emc: Replace core regulator with power >> domain >> dt-bindings: memory: tegra124: emc: Replace core regulator with power >> domain >> dt-bindings: memory: tegra20: mc: Convert to schema >> dt-bindings: memory: tegra20: emc: Convert to schema >> memory: tegra: Print out info-level once per driver probe > > Thanks, applied subset - 1-4 and 6. For patch 5/6 I expect v3. I'll make a v3, thank you.
Re: [PATCH v2 5/6] dt-bindings: memory: tegra20: emc: Convert to schema
01.04.2021 18:55, Rob Herring пишет: > On Wed, Mar 31, 2021 at 05:59:39PM +0300, Dmitry Osipenko wrote: >> 31.03.2021 16:40, Rob Herring пишет: >>> On Wed, 31 Mar 2021 02:04:44 +0300, Dmitry Osipenko wrote: >>>> Convert Tegra20 External Memory Controller binding to schema. >>>> >>>> Signed-off-by: Dmitry Osipenko >>>> --- >>>> .../memory-controllers/nvidia,tegra20-emc.txt | 130 >>>> .../nvidia,tegra20-emc.yaml | 294 ++ >>>> 2 files changed, 294 insertions(+), 130 deletions(-) >>>> delete mode 100644 >>>> Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.txt >>>> create mode 100644 >>>> Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.yaml >>>> >>> >>> My bot found errors running 'make dt_binding_check' on your patch: >>> >>> yamllint warnings/errors: >>> >>> dtschema/dtc warnings/errors: >>> Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.example.dts:33.26-55.15: >>> Warning (unit_address_vs_reg): >>> /example-0/external-memory-controller@7000f400/emc-tables@0: node has a >>> unit name, but no reg or ranges property >>> >>> See https://patchwork.ozlabs.org/patch/1460288 >>> >>> This check can fail if there are any dependencies. The base for a patch >>> series is generally the most recent rc1. >>> >>> If you already ran 'make dt_binding_check' and didn't see the above >>> error(s), then make sure 'yamllint' is installed and dt-schema is up to >>> date: >>> >>> pip3 install dtschema --upgrade >>> >>> Please check and re-submit. >>> >> >> FYI, I'm aware about this warning and I think it's fine to skip it in >> the case of this binding. > > It's not because dt_binding_check should be warning free. > >> The dtbs_check doesn't bother with that warning. > > With W=1 it will. It's off by default because there are too many of > these warnings. Patches welcome. Such warning could be silenced with some kind of a new pragma option in schema which will tell what warnings are inappropriate. But since there is no such option today, perhaps indeed should be better to just add a dummy reg property and fix the device-trees.
Re: [PATCH v1 2/2] iommu/tegra-smmu: Revert workaround that was needed for Nyan Big Chromebook
01.04.2021 11:55, Nicolin Chen пишет: > On Mon, Mar 29, 2021 at 02:32:56AM +0300, Dmitry Osipenko wrote: >> The previous commit fixes problem where display client was attaching too >> early to IOMMU during kernel boot in a multi-platform kernel configuration >> which enables CONFIG_ARM_DMA_USE_IOMMU=y. The workaround that helped to >> defer the IOMMU attachment for Nyan Big Chromebook isn't needed anymore, >> revert it. > > Sorry for the late reply. I have been busy with downstream tasks. > > I will give them a try by the end of the week. Yet, probably it'd > be better to include Guillaume also as he has the Nyan platform. > Indeed, thanks. Although, I'm pretty sure that it's the same issue which I reproduced on Nexus 7. Guillaume, could you please give a test to these patches on Nyan Big? There should be no EMEM errors in the kernel log with this patches. https://patchwork.ozlabs.org/project/linux-tegra/list/?series=236215
Re: [PATCH v4 1/6] soc/tegra: Add devm_tegra_core_dev_init_opp_table()
18.03.2021 13:37, Dmitry Osipenko пишет: > 18.03.2021 13:32, Viresh Kumar пишет: >> On 18-03-21, 13:27, Dmitry Osipenko wrote: >>> 14.03.2021 19:48, Dmitry Osipenko пишет: >>>> Add common helper which initializes OPP table for Tegra SoC core devices. >>>> >>>> Tested-by: Peter Geis # Ouya T30 >>>> Tested-by: Paul Fertser # PAZ00 T20 >>>> Tested-by: Nicolas Chauvet # PAZ00 T20 and TK1 T124 >>>> Tested-by: Matt Merhar # Ouya T30 >>>> Signed-off-by: Dmitry Osipenko >>>> --- >>>> drivers/soc/tegra/common.c | 137 + >>>> include/soc/tegra/common.h | 30 >>>> 2 files changed, 167 insertions(+) >>> >>> Viresh, do you think it will be possible to take this patch via the OPP >>> tree along with the devres patches if Thierry will give an ack? This >>> will allow us to start adding power management support to Tegra drivers >>> once 5.13 will be released. >> >> I can do that.. OR >> >> I can give an immutable to Thierry over which he can base these patches.. >> > > Thank you! > > Thierry, please let us know if you're okay with this patch and what > variant you prefer more. > It's a bit too late now for 5.13, so I'll re-send this patch later on for 5.14 separately and along with other patches that will make use of this new helper.
Re: [PATCH v2 5/6] dt-bindings: memory: tegra20: emc: Convert to schema
31.03.2021 16:40, Rob Herring пишет: > On Wed, 31 Mar 2021 02:04:44 +0300, Dmitry Osipenko wrote: >> Convert Tegra20 External Memory Controller binding to schema. >> >> Signed-off-by: Dmitry Osipenko >> --- >> .../memory-controllers/nvidia,tegra20-emc.txt | 130 >> .../nvidia,tegra20-emc.yaml | 294 ++ >> 2 files changed, 294 insertions(+), 130 deletions(-) >> delete mode 100644 >> Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.txt >> create mode 100644 >> Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.yaml >> > > My bot found errors running 'make dt_binding_check' on your patch: > > yamllint warnings/errors: > > dtschema/dtc warnings/errors: > Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.example.dts:33.26-55.15: > Warning (unit_address_vs_reg): > /example-0/external-memory-controller@7000f400/emc-tables@0: node has a unit > name, but no reg or ranges property > > See https://patchwork.ozlabs.org/patch/1460288 > > This check can fail if there are any dependencies. The base for a patch > series is generally the most recent rc1. > > If you already ran 'make dt_binding_check' and didn't see the above > error(s), then make sure 'yamllint' is installed and dt-schema is up to > date: > > pip3 install dtschema --upgrade > > Please check and re-submit. > FYI, I'm aware about this warning and I think it's fine to skip it in the case of this binding. The dtbs_check doesn't bother with that warning.
[PATCH v2 6/6] memory: tegra: Print out info-level once per driver probe
Probing of EMC drivers may be deferred and in this case we get duplicated
info messages during kernel boot. Use dev_info_once() helper to silence
the duplicated messages.
Signed-off-by: Dmitry Osipenko
---
drivers/memory/tegra/tegra124-emc.c | 12 ++--
drivers/memory/tegra/tegra20-emc.c | 20 ++--
drivers/memory/tegra/tegra30-emc.c | 18 +-
3 files changed, 25 insertions(+), 25 deletions(-)
diff --git a/drivers/memory/tegra/tegra124-emc.c
b/drivers/memory/tegra/tegra124-emc.c
index 874e1a0f23cd..5699d909abc2 100644
--- a/drivers/memory/tegra/tegra124-emc.c
+++ b/drivers/memory/tegra/tegra124-emc.c
@@ -905,7 +905,7 @@ static int emc_init(struct tegra_emc *emc)
else
emc->dram_bus_width = 32;
- dev_info(emc->dev, "%ubit DRAM bus\n", emc->dram_bus_width);
+ dev_info_once(emc->dev, "%ubit DRAM bus\n", emc->dram_bus_width);
emc->dram_type &= EMC_FBIO_CFG5_DRAM_TYPE_MASK;
emc->dram_type >>= EMC_FBIO_CFG5_DRAM_TYPE_SHIFT;
@@ -1419,8 +1419,8 @@ static int tegra_emc_opp_table_init(struct tegra_emc *emc)
goto put_hw_table;
}
- dev_info(emc->dev, "OPP HW ver. 0x%x, current clock rate %lu MHz\n",
-hw_version, clk_get_rate(emc->clk) / 100);
+ dev_info_once(emc->dev, "OPP HW ver. 0x%x, current clock rate %lu
MHz\n",
+ hw_version, clk_get_rate(emc->clk) / 100);
/* first dummy rate-set initializes voltage state */
err = dev_pm_opp_set_rate(emc->dev, clk_get_rate(emc->clk));
@@ -1475,9 +1475,9 @@ static int tegra_emc_probe(struct platform_device *pdev)
if (err)
return err;
} else {
- dev_info(>dev,
-"no memory timings for RAM code %u found in DT\n",
-ram_code);
+ dev_info_once(>dev,
+ "no memory timings for RAM code %u found in DT\n",
+ ram_code);
}
err = emc_init(emc);
diff --git a/drivers/memory/tegra/tegra20-emc.c
b/drivers/memory/tegra/tegra20-emc.c
index d653a6be8d7f..da8a0da8da79 100644
--- a/drivers/memory/tegra/tegra20-emc.c
+++ b/drivers/memory/tegra/tegra20-emc.c
@@ -411,12 +411,12 @@ static int tegra_emc_load_timings_from_dt(struct
tegra_emc *emc,
sort(emc->timings, emc->num_timings, sizeof(*timing), cmp_timings,
NULL);
- dev_info(emc->dev,
-"got %u timings for RAM code %u (min %luMHz max %luMHz)\n",
-emc->num_timings,
-tegra_read_ram_code(),
-emc->timings[0].rate / 100,
-emc->timings[emc->num_timings - 1].rate / 100);
+ dev_info_once(emc->dev,
+ "got %u timings for RAM code %u (min %luMHz max
%luMHz)\n",
+ emc->num_timings,
+ tegra_read_ram_code(),
+ emc->timings[0].rate / 100,
+ emc->timings[emc->num_timings - 1].rate / 100);
return 0;
}
@@ -429,7 +429,7 @@ tegra_emc_find_node_by_ram_code(struct device *dev)
int err;
if (of_get_child_count(dev->of_node) == 0) {
- dev_info(dev, "device-tree doesn't have memory timings\n");
+ dev_info_once(dev, "device-tree doesn't have memory timings\n");
return NULL;
}
@@ -496,7 +496,7 @@ static int emc_setup_hw(struct tegra_emc *emc)
else
emc->dram_bus_width = 32;
- dev_info(emc->dev, "%ubit DRAM bus\n", emc->dram_bus_width);
+ dev_info_once(emc->dev, "%ubit DRAM bus\n", emc->dram_bus_width);
return 0;
}
@@ -931,8 +931,8 @@ static int tegra_emc_opp_table_init(struct tegra_emc *emc)
goto put_hw_table;
}
- dev_info(emc->dev, "OPP HW ver. 0x%x, current clock rate %lu MHz\n",
-hw_version, clk_get_rate(emc->clk) / 100);
+ dev_info_once(emc->dev, "OPP HW ver. 0x%x, current clock rate %lu
MHz\n",
+ hw_version, clk_get_rate(emc->clk) / 100);
/* first dummy rate-set initializes voltage state */
err = dev_pm_opp_set_rate(emc->dev, clk_get_rate(emc->clk));
diff --git a/drivers/memory/tegra/tegra30-emc.c
b/drivers/memory/tegra/tegra30-emc.c
index 6985da0ffb35..829f6d673c96 100644
--- a/drivers/memory/tegra/tegra30-emc.c
+++ b/drivers/memory/tegra/tegra30-emc.c
@@ -998,12 +998,12 @@ static int emc_load_timings_from_dt(struct tegra_emc *emc,
if (err)
return err;
- dev_info(emc->dev,
-"got %u timings for RAM code %u (min
[PATCH v2 5/6] dt-bindings: memory: tegra20: emc: Convert to schema
Convert Tegra20 External Memory Controller binding to schema.
Signed-off-by: Dmitry Osipenko
---
.../memory-controllers/nvidia,tegra20-emc.txt | 130
.../nvidia,tegra20-emc.yaml | 294 ++
2 files changed, 294 insertions(+), 130 deletions(-)
delete mode 100644
Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.txt
create mode 100644
Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.yaml
diff --git
a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.txt
b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.txt
deleted file mode 100644
index d2250498c36d..
---
a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.txt
+++ /dev/null
@@ -1,130 +0,0 @@
-Embedded Memory Controller
-
-Properties:
-- name : Should be emc
-- #address-cells : Should be 1
-- #size-cells : Should be 0
-- compatible : Should contain "nvidia,tegra20-emc".
-- reg : Offset and length of the register set for the device
-- nvidia,use-ram-code : If present, the sub-nodes will be addressed
- and chosen using the ramcode board selector. If omitted, only one
- set of tables can be present and said tables will be used
- irrespective of ram-code configuration.
-- interrupts : Should contain EMC General interrupt.
-- clocks : Should contain EMC clock.
-- nvidia,memory-controller : Phandle of the Memory Controller node.
-- #interconnect-cells : Should be 0.
-- operating-points-v2: See ../bindings/opp/opp.txt for details.
-
-For each opp entry in 'operating-points-v2' table:
-- opp-supported-hw: One bitfield indicating SoC process ID mask
-
- A bitwise AND is performed against this value and if any bit
- matches, the OPP gets enabled.
-
-Optional properties:
-- power-domains: Phandle of the SoC "core" power domain.
-
-Child device nodes describe the memory settings for different configurations
and clock rates.
-
-Example:
-
- opp_table: opp-table {
- compatible = "operating-points-v2";
-
- opp@3600 {
- opp-microvolt = <95 95 130>;
- opp-hz = /bits/ 64 <3600>;
- };
- ...
- };
-
- memory-controller@7000f400 {
- #address-cells = < 1 >;
- #size-cells = < 0 >;
- #interconnect-cells = <0>;
- compatible = "nvidia,tegra20-emc";
- reg = <0x7000f400 0x400>;
- interrupts = <0 78 0x04>;
- clocks = <_car TEGRA20_CLK_EMC>;
- nvidia,memory-controller = <>;
- power-domains = <>;
- operating-points-v2 = <_table>;
- }
-
-
-Embedded Memory Controller ram-code table
-
-If the emc node has the nvidia,use-ram-code property present, then the
-next level of nodes below the emc table are used to specify which settings
-apply for which ram-code settings.
-
-If the emc node lacks the nvidia,use-ram-code property, this level is omitted
-and the tables are stored directly under the emc node (see below).
-
-Properties:
-
-- name : Should be emc-tables
-- nvidia,ram-code : the binary representation of the ram-code board strappings
- for which this node (and children) are valid.
-
-
-
-Embedded Memory Controller configuration table
-
-This is a table containing the EMC register settings for the various
-operating speeds of the memory controller. They are always located as
-subnodes of the emc controller node.
-
-There are two ways of specifying which tables to use:
-
-* The simplest is if there is just one set of tables in the device tree,
- and they will always be used (based on which frequency is used).
- This is the preferred method, especially when firmware can fill in
- this information based on the specific system information and just
- pass it on to the kernel.
-
-* The slightly more complex one is when more than one memory configuration
- might exist on the system. The Tegra20 platform handles this during
- early boot by selecting one out of possible 4 memory settings based
- on a 2-pin "ram code" bootstrap setting on the board. The values of
- these strappings can be read through a register in the SoC, and thus
- used to select which tables to use.
-
-Properties:
-- name : Should be emc-table
-- compatible : Should contain "nvidia,tegra20-emc-table".
-- reg : either an opaque enumerator to tell different tables apart, or
- the valid frequency for which the table should be used (in kHz).
-- clock-frequency : the clock frequency for the EMC at which this
- table should be used (in kHz).
-- nvidia,emc-registers : a 46 word array of EMC registers to be programmed
- for operation at the 'clock-frequency' setting.
- The order and contents of the registers are:
-RC, RFC,
[PATCH v2 4/6] dt-bindings: memory: tegra20: mc: Convert to schema
Convert Tegra20 Memory Controller binding to schema.
Signed-off-by: Dmitry Osipenko
---
.../memory-controllers/nvidia,tegra20-mc.txt | 40 --
.../memory-controllers/nvidia,tegra20-mc.yaml | 79 +++
2 files changed, 79 insertions(+), 40 deletions(-)
delete mode 100644
Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-mc.txt
create mode 100644
Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-mc.yaml
diff --git
a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-mc.txt
b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-mc.txt
deleted file mode 100644
index 739b7c6f2e26..
--- a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-mc.txt
+++ /dev/null
@@ -1,40 +0,0 @@
-NVIDIA Tegra20 MC(Memory Controller)
-
-Required properties:
-- compatible : "nvidia,tegra20-mc-gart"
-- reg : Should contain 2 register ranges: physical base address and length of
- the controller's registers and the GART aperture respectively.
-- clocks: Must contain an entry for each entry in clock-names.
- See ../clocks/clock-bindings.txt for details.
-- clock-names: Must include the following entries:
- - mc: the module's clock input
-- interrupts : Should contain MC General interrupt.
-- #reset-cells : Should be 1. This cell represents memory client module ID.
- The assignments may be found in header file
- or in the TRM documentation.
-- #iommu-cells: Should be 0. This cell represents the number of cells in an
- IOMMU specifier needed to encode an address. GART supports only a single
- address space that is shared by all devices, therefore no additional
- information needed for the address encoding.
-- #interconnect-cells : Should be 1. This cell represents memory client.
- The assignments may be found in header file
.
-
-Example:
- mc: memory-controller@7000f000 {
- compatible = "nvidia,tegra20-mc-gart";
- reg = <0x7000f000 0x400 /* controller registers */
- 0x5800 0x0200>; /* GART aperture */
- clocks = <_car TEGRA20_CLK_MC>;
- clock-names = "mc";
- interrupts = ;
- #reset-cells = <1>;
- #iommu-cells = <0>;
- #interconnect-cells = <1>;
- };
-
- video-codec@6001a000 {
- compatible = "nvidia,tegra20-vde";
- ...
- resets = < TEGRA20_MC_RESET_VDE>;
- iommus = <>;
- };
diff --git
a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-mc.yaml
b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-mc.yaml
new file mode 100644
index ..55caf6905399
--- /dev/null
+++
b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-mc.yaml
@@ -0,0 +1,79 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/memory-controllers/nvidia,tegra20-mc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NVIDIA Tegra20 SoC Memory Controller
+
+maintainers:
+ - Dmitry Osipenko
+ - Jon Hunter
+ - Thierry Reding
+
+description: |
+ The Tegra20 Memory Controller merges request streams from various client
+ interfaces into request stream(s) for the various memory target devices,
+ and returns response data to the various clients. The Memory Controller
+ has a configurable arbitration algorithm to allow the user to fine-tune
+ performance among the various clients.
+
+ Tegra20 Memory Controller includes the GART (Graphics Address Relocation
+ Table) which allows Memory Controller to provide a linear view of a
+ fragmented memory pages.
+
+properties:
+ compatible:
+const: nvidia,tegra20-mc-gart
+
+ reg:
+items:
+ - description: controller registers
+ - description: GART registers
+
+ clocks:
+maxItems: 1
+
+ clock-names:
+items:
+ - const: mc
+
+ interrupts:
+maxItems: 1
+
+ "#reset-cells":
+const: 1
+
+ "#iommu-cells":
+const: 0
+
+ "#interconnect-cells":
+const: 1
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - clocks
+ - clock-names
+ - "#reset-cells"
+ - "#iommu-cells"
+ - "#interconnect-cells"
+
+additionalProperties: false
+
+examples:
+ - |
+memory-controller@7000f000 {
+compatible = "nvidia,tegra20-mc-gart";
+reg = <0x7000f000 0x400>, /* Controller registers */
+ <0x5800 0x0200>; /* GART aperture */
+clocks = <_controller 32>;
+clock-names = "mc";
+
+interrupts = <0 77 4>;
+
+#iommu-cells = <0>;
+#reset-cells = <1>;
+#interconnect-cells = <1>;
+};
--
2.30.2
[PATCH v2 3/6] dt-bindings: memory: tegra124: emc: Replace core regulator with power domain
Power domain fits much better than a voltage regulator in regards to a proper hardware description and from a software perspective as well. Hence replace the core regulator with the power domain. Note that this doesn't affect any existing DTBs because we haven't started to use the regulator yet, and thus, it's okay to change it. Signed-off-by: Dmitry Osipenko --- .../bindings/memory-controllers/nvidia,tegra124-emc.yaml | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra124-emc.yaml b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra124-emc.yaml index 09bde65e1955..9163c3f12a85 100644 --- a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra124-emc.yaml +++ b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra124-emc.yaml @@ -37,9 +37,10 @@ properties: description: phandle of the memory controller node - core-supply: + power-domains: +maxItems: 1 description: - Phandle of voltage regulator of the SoC "core" power domain. + Phandle of the SoC "core" power domain. operating-points-v2: description: @@ -370,7 +371,7 @@ examples: nvidia,memory-controller = <>; operating-points-v2 = <_opp_table>; -core-supply = <_core>; +power-domains = <>; #interconnect-cells = <0>; -- 2.30.2
[PATCH v2 0/6] NVIDIA Tegra memory improvements
Hi, This series replaces the raw voltage regulator with a power domain that will be managing SoC core voltage. The core power domain patches are still under review, but it's clear at this point that this is the way we will implement the DVFS support. The remaining Tegra20 memory bindings are converted to schema. I also made a small improvement to the memory drivers. Changelog: v2: - Fixed typos in the converted schemas. - Corrected reg entry of tegra20-mc-gart schema to use fixed number of items. - Made power-domain to use maxItems instead of $ref phandle in schemas. Dmitry Osipenko (6): dt-bindings: memory: tegra20: emc: Replace core regulator with power domain dt-bindings: memory: tegra30: emc: Replace core regulator with power domain dt-bindings: memory: tegra124: emc: Replace core regulator with power domain dt-bindings: memory: tegra20: mc: Convert to schema dt-bindings: memory: tegra20: emc: Convert to schema memory: tegra: Print out info-level once per driver probe .../nvidia,tegra124-emc.yaml | 7 +- .../memory-controllers/nvidia,tegra20-emc.txt | 130 .../nvidia,tegra20-emc.yaml | 294 ++ .../memory-controllers/nvidia,tegra20-mc.txt | 40 --- .../memory-controllers/nvidia,tegra20-mc.yaml | 79 + .../nvidia,tegra30-emc.yaml | 7 +- drivers/memory/tegra/tegra124-emc.c | 12 +- drivers/memory/tegra/tegra20-emc.c| 20 +- drivers/memory/tegra/tegra30-emc.c| 18 +- 9 files changed, 406 insertions(+), 201 deletions(-) delete mode 100644 Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.txt create mode 100644 Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.yaml delete mode 100644 Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-mc.txt create mode 100644 Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-mc.yaml -- 2.30.2
[PATCH v2 2/6] dt-bindings: memory: tegra30: emc: Replace core regulator with power domain
Power domain fits much better than a voltage regulator in regards to a proper hardware description and from a software perspective as well. Hence replace the core regulator with the power domain. Note that this doesn't affect any existing DTBs because we haven't started to use the regulator yet, and thus, it's okay to change it. Signed-off-by: Dmitry Osipenko --- .../bindings/memory-controllers/nvidia,tegra30-emc.yaml| 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-emc.yaml b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-emc.yaml index 0a2e2c0d0fdd..fb6af14cb49c 100644 --- a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-emc.yaml +++ b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-emc.yaml @@ -39,9 +39,10 @@ properties: description: Phandle of the Memory Controller node. - core-supply: + power-domains: +maxItems: 1 description: - Phandle of voltage regulator of the SoC "core" power domain. + Phandle of the SoC "core" power domain. operating-points-v2: description: @@ -241,7 +242,7 @@ examples: nvidia,memory-controller = <>; operating-points-v2 = <_opp_table>; -core-supply = <_core>; +power-domains = <>; #interconnect-cells = <0>; -- 2.30.2
[PATCH v2 1/6] dt-bindings: memory: tegra20: emc: Replace core regulator with power domain
Power domain fits much better than a voltage regulator in regards to a proper hardware description and from a software perspective as well. Hence replace the core regulator with the power domain. Note that this doesn't affect any existing DTBs because we haven't started to use the regulator yet, and thus, it's okay to change it. Reviewed-by: Rob Herring Signed-off-by: Dmitry Osipenko --- .../bindings/memory-controllers/nvidia,tegra20-emc.txt| 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.txt b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.txt index cc443fcf4bec..d2250498c36d 100644 --- a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.txt +++ b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.txt @@ -23,7 +23,7 @@ For each opp entry in 'operating-points-v2' table: matches, the OPP gets enabled. Optional properties: -- core-supply: Phandle of voltage regulator of the SoC "core" power domain. +- power-domains: Phandle of the SoC "core" power domain. Child device nodes describe the memory settings for different configurations and clock rates. @@ -48,7 +48,7 @@ Example: interrupts = <0 78 0x04>; clocks = <_car TEGRA20_CLK_EMC>; nvidia,memory-controller = <>; - core-supply = <_vdd_reg>; + power-domains = <>; operating-points-v2 = <_table>; } -- 2.30.2
Re: [PATCH v1 2/6] dt-bindings: memory: tegra30: emc: Replace core regulator with power domain
31.03.2021 01:23, Rob Herring пишет: > On Mon, Mar 29, 2021 at 10:45:58PM +0300, Dmitry Osipenko wrote: >> Power domain fits much better than a voltage regulator in regards to >> a proper hardware description and from a software perspective as well. >> Hence replace the core regulator with the power domain. Note that this >> doesn't affect any existing DTBs because we haven't started to use the >> regulator yet, and thus, it's okay to change it. >> >> Signed-off-by: Dmitry Osipenko >> --- >> .../bindings/memory-controllers/nvidia,tegra30-emc.yaml| 7 --- >> 1 file changed, 4 insertions(+), 3 deletions(-) >> >> diff --git >> a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-emc.yaml >> >> b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-emc.yaml >> index 0a2e2c0d0fdd..4a2edb9b8bdc 100644 >> --- >> a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-emc.yaml >> +++ >> b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-emc.yaml >> @@ -39,9 +39,10 @@ properties: >> description: >>Phandle of the Memory Controller node. >> >> - core-supply: >> + power-domains: >> +$ref: /schemas/types.yaml#/definitions/phandle > > 'power-domains' already has a type. We need to know how many (maxItems). Alright, I see now what makes the difference. Thank you and Krzysztof for the suggestion.
Re: [PATCH v5 3/5] i2c: add support for HiSilicon I2C controller
30.03.2021 17:19, Yicong Yang пишет: > Add HiSilicon I2C controller driver for the Kunpeng SoC. It provides > the access to the i2c busses, which connects to the eeprom, rtc, etc. > > The driver works with IRQ mode, and supports basic I2C features and 10bit > address. The DMA is not supported. > > Reviewed-by: Andy Shevchenko > Signed-off-by: Yicong Yang > --- > drivers/i2c/busses/Kconfig| 10 + > drivers/i2c/busses/Makefile | 1 + > drivers/i2c/busses/i2c-hisi.c | 506 > ++ > 3 files changed, 517 insertions(+) > create mode 100644 drivers/i2c/busses/i2c-hisi.c Reviewed-by: Dmitry Osipenko
Re: [PATCH v5 3/5] i2c: add support for HiSilicon I2C controller
30.03.2021 19:24, Dmitry Osipenko пишет:
>> +struct hisi_i2c_controller {
>> +struct i2c_adapter adapter;
>> +void __iomem *iobase;
>> +struct device *dev;
>> +int irq;
>> +
>> +/* Intermediates for recording the transfer process */
>> +struct completion *completion;
>> +struct i2c_msg *msgs;
>> +int msg_num;
>> +int msg_tx_idx;
>> +int buf_tx_idx;
>> +int msg_rx_idx;
>> +int buf_rx_idx;
>> +u16 tar_addr;
>> +u32 xfer_err;
>> +
>> +/* I2C bus configuration */
>> +struct i2c_timings t;
>> +u64 clk_rate_khz;
> Looks like the u64 is overkill here and it doesn't allow you to use
> COMPILE_TEST because 32bit arches can divide u64 only using the
> do_div(), please fix this.
I see now that the value isn't divided anywhere directly in the code, my
bad. Looks good then.
Re: [PATCH v5 1/5] i2c: core: add managed function for adding i2c adapters
30.03.2021 17:19, Yicong Yang пишет:
> Some I2C controller drivers will only unregister the I2C
> adapter in their .remove() callback, which can be done
> by simply using a managed variant to add the I2C adapter.
>
> So add the managed functions for adding the I2C adapter.
>
> Reviewed-by: Andy Shevchenko
> Signed-off-by: Yicong Yang
> ---
> drivers/i2c/i2c-core-base.c | 26 ++
> include/linux/i2c.h | 1 +
> 2 files changed, 27 insertions(+)
>
> diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
> index 63ebf72..de9402c 100644
> --- a/drivers/i2c/i2c-core-base.c
> +++ b/drivers/i2c/i2c-core-base.c
> @@ -1703,6 +1703,32 @@ void i2c_del_adapter(struct i2c_adapter *adap)
> }
> EXPORT_SYMBOL(i2c_del_adapter);
>
> +static void devm_i2c_del_adapter(void *adapter)
> +{
> + i2c_del_adapter(adapter);
> +}
> +
> +/**
> + * devm_i2c_add_adapter - device-managed variant of i2c_add_adapter()
> + * @dev: managing device for adding this I2C adapter
> + * @adapter: the adapter to add
> + * Context: can sleep
> + *
> + * Add adapter with dynamic bus number, same with i2c_add_adapter()
> + * but the adapter will be auto deleted on driver detach.
> + */
> +int devm_i2c_add_adapter(struct device *dev, struct i2c_adapter *adapter)
> +{
> + int ret;
> +
> + ret = i2c_add_adapter(adapter);
> + if (ret)
> + return ret;
> +
> + return devm_add_action_or_reset(dev, devm_i2c_del_adapter, adapter);
> +}
> +EXPORT_SYMBOL_GPL(devm_i2c_add_adapter);
> +
> static void i2c_parse_timing(struct device *dev, char *prop_name, u32
> *cur_val_p,
> u32 def_val, bool use_def)
> {
> diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> index 5662265..10bd0b0 100644
> --- a/include/linux/i2c.h
> +++ b/include/linux/i2c.h
> @@ -844,6 +844,7 @@ static inline void i2c_mark_adapter_resumed(struct
> i2c_adapter *adap)
> */
> #if IS_ENABLED(CONFIG_I2C)
> int i2c_add_adapter(struct i2c_adapter *adap);
> +int devm_i2c_add_adapter(struct device *dev, struct i2c_adapter *adapter);
> void i2c_del_adapter(struct i2c_adapter *adap);
> int i2c_add_numbered_adapter(struct i2c_adapter *adap);
>
>
Reviewed-by: Dmitry Osipenko
Re: [PATCH v5 3/5] i2c: add support for HiSilicon I2C controller
30.03.2021 17:19, Yicong Yang пишет:
...
> +struct hisi_i2c_controller {
> + struct i2c_adapter adapter;
> + void __iomem *iobase;
> + struct device *dev;
> + int irq;
> +
> + /* Intermediates for recording the transfer process */
> + struct completion *completion;
> + struct i2c_msg *msgs;
> + int msg_num;
> + int msg_tx_idx;
> + int buf_tx_idx;
> + int msg_rx_idx;
> + int buf_rx_idx;
> + u16 tar_addr;
> + u32 xfer_err;
> +
> + /* I2C bus configuration */
> + struct i2c_timings t;
> + u64 clk_rate_khz;
Looks like the u64 is overkill here and it doesn't allow you to use
COMPILE_TEST because 32bit arches can divide u64 only using the
do_div(), please fix this.
...
> +static const struct acpi_device_id hisi_i2c_acpi_ids[] = {
> + { "HISI03D1", 0 },
> + { }
> +};
> +MODULE_DEVICE_TABLE(acpi, hisi_i2c_acpi_ids);
> +MODULE_ALIAS("platform:hisi-i2c");
Nit: I think the MODULE_ALIAS shouldn't be necessary for this driver
since it will be matched by the ACPI table. You should be able to remove
it safely.
Re: [PATCH v1 5/6] dt-bindings: memory: tegra20: emc: Convert to schema
30.03.2021 18:29, Dmitry Osipenko пишет: > 30.03.2021 11:48, Krzysztof Kozlowski пишет: >>> + power-domains: >>> +$ref: /schemas/types.yaml#/definitions/phandle >>> +description: >>> + Phandle of the SoC "core" power domain. >> I think the core checks the type, so you only need to limit max items. >> > > It's a bit confusing that both variants work and it's not apparent what > variant is better. > > I actually used the max items limit initially and then changed it to > $ref phandle because it appeared to me that it's a better choice. I'll > switch back to the limit in v2, thanks. > Although, I'm still not sure what is the best variant. Could you please clarify why maxItems is better? Seems the $ref phandle already limits domain items to 1. So I don't understand what's the difference.
Re: [PATCH v6 0/7] Couple improvements for Tegra clk driver
20.03.2021 18:26, Dmitry Osipenko пишет: > This series fixes couple minor standalone problems of the Tegra clk > driver. Hello Stephen, Do you have any objects if Thierry will take this series into the Tegra tree? Or will you be able to take the patches into the clk tree? Please let us know what works best for you, thanks in advance.
Re: [PATCH v1 5/6] dt-bindings: memory: tegra20: emc: Convert to schema
30.03.2021 11:48, Krzysztof Kozlowski пишет: >> + "^emc-tables@[a-z0-9\\-]+$": > Why \ and - in the pattern? Good catch, I thought that '-' needs to be escaped, but then forgot to remove the unnecessary slashes.
Re: [PATCH v1 5/6] dt-bindings: memory: tegra20: emc: Convert to schema
30.03.2021 11:48, Krzysztof Kozlowski пишет: >> + nvidia,use-ram-code: >> +type: boolean >> +description: >> + If present, the emc-tables@ sub-nodes will be addressed. >> + >> +patternProperties: >> + "^emc-table@[0-9]+$": > This might not be easy but you should add constraints when emc-table and > emc-tables are expected. The schema should check if proper node is used > depending on "nvidia,use-ram-code". > I'm afraid this is not doable. If you have an example how to do this, please share it with me. I see that there is a "dependencies:", but it doesn't work with the patterns, IIUC.
Re: [PATCH v1 5/6] dt-bindings: memory: tegra20: emc: Convert to schema
30.03.2021 11:48, Krzysztof Kozlowski пишет: >> + power-domains: >> +$ref: /schemas/types.yaml#/definitions/phandle >> +description: >> + Phandle of the SoC "core" power domain. > I think the core checks the type, so you only need to limit max items. > It's a bit confusing that both variants work and it's not apparent what variant is better. I actually used the max items limit initially and then changed it to $ref phandle because it appeared to me that it's a better choice. I'll switch back to the limit in v2, thanks.
Re: [PATCH v1 4/6] dt-bindings: memory: tegra20: mc: Convert to schema
30.03.2021 11:37, Krzysztof Kozlowski пишет: >> +properties: >> + compatible: >> +const: nvidia,tegra20-mc-gart >> + >> + reg: >> +minItems: 1 >> +maxItems: 2 > I think you always need two regs, don't you? If so, then better to use > "description" like in > Documentation/devicetree/bindings/example-schema.yaml to describe which > set is for which range/purpose. I did this because the original example from the txt binding was failing with: Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-mc.example.dt.yaml: memory-controller@7000f000: reg: [[1879109632, 1024, 1476395008, 33554432]] is too short But then I just corrected the example and forgot to change the reg entry. I'll fix it in v2, thanks.
Re: [PATCH v1 4/6] dt-bindings: memory: tegra20: mc: Convert to schema
30.03.2021 16:46, Rob Herring пишет: > On Tue, Mar 30, 2021 at 08:08:43AM -0500, Rob Herring wrote: >> On Mon, 29 Mar 2021 22:46:00 +0300, Dmitry Osipenko wrote: >>> Convert Tegra20 Memory Controller binding to schema. >>> >>> Signed-off-by: Dmitry Osipenko >>> --- >>> .../memory-controllers/nvidia,tegra20-mc.txt | 40 -- >>> .../memory-controllers/nvidia,tegra20-mc.yaml | 78 +++ >>> 2 files changed, 78 insertions(+), 40 deletions(-) >>> delete mode 100644 >>> Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-mc.txt >>> create mode 100644 >>> Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-mc.yaml >>> >> >> My bot found errors running 'make dt_binding_check' on your patch: >> >> yamllint warnings/errors: >> >> dtschema/dtc warnings/errors: >> Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-mc.example.dt.yaml:0:0: >> /example-0/memory-controller@7000f000: failed to match any schema with >> compatible: ['nvidia,tegra20-mc'] > > Yes, this is a new warning. It's off by default for dt_binding_check > until we fix the existing warnings, but you can enable by adding > 'DT_CHECKER_FLAGS=-m'. Support for this is in the dt/next branch. Thanks!
[PATCH v1] i2c: Make i2c_recover_bus() to return -EBUSY if bus recovery unimplemented
The i2c_recover_bus() returns -EOPNOTSUPP if bus recovery isn't wired up
by the bus driver, which the case for Tegra I2C driver for example. This
error code is then propagated to I2C client and might be confusing, thus
make i2c_recover_bus() to return -EBUSY instead.
Suggested-by: Wolfram Sang
Signed-off-by: Dmitry Osipenko
---
drivers/i2c/i2c-core-base.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index f21362355973..7039b8a06f3a 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -249,7 +249,7 @@ EXPORT_SYMBOL_GPL(i2c_generic_scl_recovery);
int i2c_recover_bus(struct i2c_adapter *adap)
{
if (!adap->bus_recovery_info)
- return -EOPNOTSUPP;
+ return -EBUSY;
dev_dbg(>dev, "Trying i2c bus recovery\n");
return adap->bus_recovery_info->recover_bus(adap);
--
2.30.2
[PATCH v1 5/6] dt-bindings: memory: tegra20: emc: Convert to schema
Convert Tegra20 External Memory Controller binding to schema.
Signed-off-by: Dmitry Osipenko
---
.../memory-controllers/nvidia,tegra20-emc.txt | 130
.../nvidia,tegra20-emc.yaml | 294 ++
2 files changed, 294 insertions(+), 130 deletions(-)
delete mode 100644
Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.txt
create mode 100644
Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.yaml
diff --git
a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.txt
b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.txt
deleted file mode 100644
index d2250498c36d..
---
a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.txt
+++ /dev/null
@@ -1,130 +0,0 @@
-Embedded Memory Controller
-
-Properties:
-- name : Should be emc
-- #address-cells : Should be 1
-- #size-cells : Should be 0
-- compatible : Should contain "nvidia,tegra20-emc".
-- reg : Offset and length of the register set for the device
-- nvidia,use-ram-code : If present, the sub-nodes will be addressed
- and chosen using the ramcode board selector. If omitted, only one
- set of tables can be present and said tables will be used
- irrespective of ram-code configuration.
-- interrupts : Should contain EMC General interrupt.
-- clocks : Should contain EMC clock.
-- nvidia,memory-controller : Phandle of the Memory Controller node.
-- #interconnect-cells : Should be 0.
-- operating-points-v2: See ../bindings/opp/opp.txt for details.
-
-For each opp entry in 'operating-points-v2' table:
-- opp-supported-hw: One bitfield indicating SoC process ID mask
-
- A bitwise AND is performed against this value and if any bit
- matches, the OPP gets enabled.
-
-Optional properties:
-- power-domains: Phandle of the SoC "core" power domain.
-
-Child device nodes describe the memory settings for different configurations
and clock rates.
-
-Example:
-
- opp_table: opp-table {
- compatible = "operating-points-v2";
-
- opp@3600 {
- opp-microvolt = <95 95 130>;
- opp-hz = /bits/ 64 <3600>;
- };
- ...
- };
-
- memory-controller@7000f400 {
- #address-cells = < 1 >;
- #size-cells = < 0 >;
- #interconnect-cells = <0>;
- compatible = "nvidia,tegra20-emc";
- reg = <0x7000f400 0x400>;
- interrupts = <0 78 0x04>;
- clocks = <_car TEGRA20_CLK_EMC>;
- nvidia,memory-controller = <>;
- power-domains = <>;
- operating-points-v2 = <_table>;
- }
-
-
-Embedded Memory Controller ram-code table
-
-If the emc node has the nvidia,use-ram-code property present, then the
-next level of nodes below the emc table are used to specify which settings
-apply for which ram-code settings.
-
-If the emc node lacks the nvidia,use-ram-code property, this level is omitted
-and the tables are stored directly under the emc node (see below).
-
-Properties:
-
-- name : Should be emc-tables
-- nvidia,ram-code : the binary representation of the ram-code board strappings
- for which this node (and children) are valid.
-
-
-
-Embedded Memory Controller configuration table
-
-This is a table containing the EMC register settings for the various
-operating speeds of the memory controller. They are always located as
-subnodes of the emc controller node.
-
-There are two ways of specifying which tables to use:
-
-* The simplest is if there is just one set of tables in the device tree,
- and they will always be used (based on which frequency is used).
- This is the preferred method, especially when firmware can fill in
- this information based on the specific system information and just
- pass it on to the kernel.
-
-* The slightly more complex one is when more than one memory configuration
- might exist on the system. The Tegra20 platform handles this during
- early boot by selecting one out of possible 4 memory settings based
- on a 2-pin "ram code" bootstrap setting on the board. The values of
- these strappings can be read through a register in the SoC, and thus
- used to select which tables to use.
-
-Properties:
-- name : Should be emc-table
-- compatible : Should contain "nvidia,tegra20-emc-table".
-- reg : either an opaque enumerator to tell different tables apart, or
- the valid frequency for which the table should be used (in kHz).
-- clock-frequency : the clock frequency for the EMC at which this
- table should be used (in kHz).
-- nvidia,emc-registers : a 46 word array of EMC registers to be programmed
- for operation at the 'clock-frequency' setting.
- The order and contents of the registers are:
-RC, RFC,
[PATCH v1 3/6] dt-bindings: memory: tegra124: emc: Replace core regulator with power domain
Power domain fits much better than a voltage regulator in regards to a proper hardware description and from a software perspective as well. Hence replace the core regulator with the power domain. Note that this doesn't affect any existing DTBs because we haven't started to use the regulator yet, and thus, it's okay to change it. Signed-off-by: Dmitry Osipenko --- .../bindings/memory-controllers/nvidia,tegra124-emc.yaml | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra124-emc.yaml b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra124-emc.yaml index 09bde65e1955..a7483547ccf8 100644 --- a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra124-emc.yaml +++ b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra124-emc.yaml @@ -37,9 +37,10 @@ properties: description: phandle of the memory controller node - core-supply: + power-domains: +$ref: /schemas/types.yaml#/definitions/phandle description: - Phandle of voltage regulator of the SoC "core" power domain. + Phandle of the SoC "core" power domain. operating-points-v2: description: @@ -370,7 +371,7 @@ examples: nvidia,memory-controller = <>; operating-points-v2 = <_opp_table>; -core-supply = <_core>; +power-domains = <>; #interconnect-cells = <0>; -- 2.30.2
[PATCH v1 4/6] dt-bindings: memory: tegra20: mc: Convert to schema
Convert Tegra20 Memory Controller binding to schema.
Signed-off-by: Dmitry Osipenko
---
.../memory-controllers/nvidia,tegra20-mc.txt | 40 --
.../memory-controllers/nvidia,tegra20-mc.yaml | 78 +++
2 files changed, 78 insertions(+), 40 deletions(-)
delete mode 100644
Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-mc.txt
create mode 100644
Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-mc.yaml
diff --git
a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-mc.txt
b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-mc.txt
deleted file mode 100644
index 739b7c6f2e26..
--- a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-mc.txt
+++ /dev/null
@@ -1,40 +0,0 @@
-NVIDIA Tegra20 MC(Memory Controller)
-
-Required properties:
-- compatible : "nvidia,tegra20-mc-gart"
-- reg : Should contain 2 register ranges: physical base address and length of
- the controller's registers and the GART aperture respectively.
-- clocks: Must contain an entry for each entry in clock-names.
- See ../clocks/clock-bindings.txt for details.
-- clock-names: Must include the following entries:
- - mc: the module's clock input
-- interrupts : Should contain MC General interrupt.
-- #reset-cells : Should be 1. This cell represents memory client module ID.
- The assignments may be found in header file
- or in the TRM documentation.
-- #iommu-cells: Should be 0. This cell represents the number of cells in an
- IOMMU specifier needed to encode an address. GART supports only a single
- address space that is shared by all devices, therefore no additional
- information needed for the address encoding.
-- #interconnect-cells : Should be 1. This cell represents memory client.
- The assignments may be found in header file
.
-
-Example:
- mc: memory-controller@7000f000 {
- compatible = "nvidia,tegra20-mc-gart";
- reg = <0x7000f000 0x400 /* controller registers */
- 0x5800 0x0200>; /* GART aperture */
- clocks = <_car TEGRA20_CLK_MC>;
- clock-names = "mc";
- interrupts = ;
- #reset-cells = <1>;
- #iommu-cells = <0>;
- #interconnect-cells = <1>;
- };
-
- video-codec@6001a000 {
- compatible = "nvidia,tegra20-vde";
- ...
- resets = < TEGRA20_MC_RESET_VDE>;
- iommus = <>;
- };
diff --git
a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-mc.yaml
b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-mc.yaml
new file mode 100644
index ..c5731fa41e83
--- /dev/null
+++
b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-mc.yaml
@@ -0,0 +1,78 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/memory-controllers/nvidia,tegra20-mc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NVIDIA Tegra20 SoC Memory Controller
+
+maintainers:
+ - Dmitry Osipenko
+ - Jon Hunter
+ - Thierry Reding
+
+description: |
+ The Tegra20 Memory Controller merges request streams from various client
+ interfaces into request stream(s) for the various memory target devices,
+ and returns response data to the various clients. The Memory Controller
+ has a configurable arbitration algorithm to allow the user to fine-tune
+ performance among the various clients.
+
+ Tegra20 Memory Controller includes the GART (Graphics Address Relocation
+ Table) which allows Memory Controller to provide a linear view of a
+ fragmented memory pages.
+
+properties:
+ compatible:
+const: nvidia,tegra20-mc-gart
+
+ reg:
+minItems: 1
+maxItems: 2
+
+ clocks:
+maxItems: 1
+
+ clock-names:
+items:
+ - const: mc
+
+ interrupts:
+maxItems: 1
+
+ "#reset-cells":
+const: 1
+
+ "#iommu-cells":
+const: 0
+
+ "#interconnect-cells":
+const: 1
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - clocks
+ - clock-names
+ - "#reset-cells"
+ - "#iommu-cells"
+ - "#interconnect-cells"
+
+additionalProperties: false
+
+examples:
+ - |
+memory-controller@7000f000 {
+compatible = "nvidia,tegra20-mc";
+reg = <0x7000f000 0x400>, /* Controller registers */
+ <0x5800 0x0200>; /* GART aperture */
+clocks = <_controller 32>;
+clock-names = "mc";
+
+interrupts = <0 77 4>;
+
+#iommu-cells = <0>;
+#reset-cells = <1>;
+#interconnect-cells = <1>;
+};
--
2.30.2
[PATCH v1 6/6] memory: tegra: Print out info-level once per driver probe
Probing of EMC drivers may be deferred and in this case we get duplicated
info messages during kernel boot. Use dev_info_once() helper to silence
the duplicated messages.
Signed-off-by: Dmitry Osipenko
---
drivers/memory/tegra/tegra124-emc.c | 12 ++--
drivers/memory/tegra/tegra20-emc.c | 20 ++--
drivers/memory/tegra/tegra30-emc.c | 18 +-
3 files changed, 25 insertions(+), 25 deletions(-)
diff --git a/drivers/memory/tegra/tegra124-emc.c
b/drivers/memory/tegra/tegra124-emc.c
index 874e1a0f23cd..5699d909abc2 100644
--- a/drivers/memory/tegra/tegra124-emc.c
+++ b/drivers/memory/tegra/tegra124-emc.c
@@ -905,7 +905,7 @@ static int emc_init(struct tegra_emc *emc)
else
emc->dram_bus_width = 32;
- dev_info(emc->dev, "%ubit DRAM bus\n", emc->dram_bus_width);
+ dev_info_once(emc->dev, "%ubit DRAM bus\n", emc->dram_bus_width);
emc->dram_type &= EMC_FBIO_CFG5_DRAM_TYPE_MASK;
emc->dram_type >>= EMC_FBIO_CFG5_DRAM_TYPE_SHIFT;
@@ -1419,8 +1419,8 @@ static int tegra_emc_opp_table_init(struct tegra_emc *emc)
goto put_hw_table;
}
- dev_info(emc->dev, "OPP HW ver. 0x%x, current clock rate %lu MHz\n",
-hw_version, clk_get_rate(emc->clk) / 100);
+ dev_info_once(emc->dev, "OPP HW ver. 0x%x, current clock rate %lu
MHz\n",
+ hw_version, clk_get_rate(emc->clk) / 100);
/* first dummy rate-set initializes voltage state */
err = dev_pm_opp_set_rate(emc->dev, clk_get_rate(emc->clk));
@@ -1475,9 +1475,9 @@ static int tegra_emc_probe(struct platform_device *pdev)
if (err)
return err;
} else {
- dev_info(>dev,
-"no memory timings for RAM code %u found in DT\n",
-ram_code);
+ dev_info_once(>dev,
+ "no memory timings for RAM code %u found in DT\n",
+ ram_code);
}
err = emc_init(emc);
diff --git a/drivers/memory/tegra/tegra20-emc.c
b/drivers/memory/tegra/tegra20-emc.c
index d653a6be8d7f..da8a0da8da79 100644
--- a/drivers/memory/tegra/tegra20-emc.c
+++ b/drivers/memory/tegra/tegra20-emc.c
@@ -411,12 +411,12 @@ static int tegra_emc_load_timings_from_dt(struct
tegra_emc *emc,
sort(emc->timings, emc->num_timings, sizeof(*timing), cmp_timings,
NULL);
- dev_info(emc->dev,
-"got %u timings for RAM code %u (min %luMHz max %luMHz)\n",
-emc->num_timings,
-tegra_read_ram_code(),
-emc->timings[0].rate / 100,
-emc->timings[emc->num_timings - 1].rate / 100);
+ dev_info_once(emc->dev,
+ "got %u timings for RAM code %u (min %luMHz max
%luMHz)\n",
+ emc->num_timings,
+ tegra_read_ram_code(),
+ emc->timings[0].rate / 100,
+ emc->timings[emc->num_timings - 1].rate / 100);
return 0;
}
@@ -429,7 +429,7 @@ tegra_emc_find_node_by_ram_code(struct device *dev)
int err;
if (of_get_child_count(dev->of_node) == 0) {
- dev_info(dev, "device-tree doesn't have memory timings\n");
+ dev_info_once(dev, "device-tree doesn't have memory timings\n");
return NULL;
}
@@ -496,7 +496,7 @@ static int emc_setup_hw(struct tegra_emc *emc)
else
emc->dram_bus_width = 32;
- dev_info(emc->dev, "%ubit DRAM bus\n", emc->dram_bus_width);
+ dev_info_once(emc->dev, "%ubit DRAM bus\n", emc->dram_bus_width);
return 0;
}
@@ -931,8 +931,8 @@ static int tegra_emc_opp_table_init(struct tegra_emc *emc)
goto put_hw_table;
}
- dev_info(emc->dev, "OPP HW ver. 0x%x, current clock rate %lu MHz\n",
-hw_version, clk_get_rate(emc->clk) / 100);
+ dev_info_once(emc->dev, "OPP HW ver. 0x%x, current clock rate %lu
MHz\n",
+ hw_version, clk_get_rate(emc->clk) / 100);
/* first dummy rate-set initializes voltage state */
err = dev_pm_opp_set_rate(emc->dev, clk_get_rate(emc->clk));
diff --git a/drivers/memory/tegra/tegra30-emc.c
b/drivers/memory/tegra/tegra30-emc.c
index 6985da0ffb35..829f6d673c96 100644
--- a/drivers/memory/tegra/tegra30-emc.c
+++ b/drivers/memory/tegra/tegra30-emc.c
@@ -998,12 +998,12 @@ static int emc_load_timings_from_dt(struct tegra_emc *emc,
if (err)
return err;
- dev_info(emc->dev,
-"got %u timings for RAM code %u (min
[PATCH v1 2/6] dt-bindings: memory: tegra30: emc: Replace core regulator with power domain
Power domain fits much better than a voltage regulator in regards to a proper hardware description and from a software perspective as well. Hence replace the core regulator with the power domain. Note that this doesn't affect any existing DTBs because we haven't started to use the regulator yet, and thus, it's okay to change it. Signed-off-by: Dmitry Osipenko --- .../bindings/memory-controllers/nvidia,tegra30-emc.yaml| 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-emc.yaml b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-emc.yaml index 0a2e2c0d0fdd..4a2edb9b8bdc 100644 --- a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-emc.yaml +++ b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-emc.yaml @@ -39,9 +39,10 @@ properties: description: Phandle of the Memory Controller node. - core-supply: + power-domains: +$ref: /schemas/types.yaml#/definitions/phandle description: - Phandle of voltage regulator of the SoC "core" power domain. + Phandle of the SoC "core" power domain. operating-points-v2: description: @@ -241,7 +242,7 @@ examples: nvidia,memory-controller = <>; operating-points-v2 = <_opp_table>; -core-supply = <_core>; +power-domains = <>; #interconnect-cells = <0>; -- 2.30.2
[PATCH v1 1/6] dt-bindings: memory: tegra20: emc: Replace core regulator with power domain
Power domain fits much better than a voltage regulator in regards to a proper hardware description and from a software perspective as well. Hence replace the core regulator with the power domain. Note that this doesn't affect any existing DTBs because we haven't started to use the regulator yet, and thus, it's okay to change it. Reviewed-by: Rob Herring Signed-off-by: Dmitry Osipenko --- .../bindings/memory-controllers/nvidia,tegra20-emc.txt| 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.txt b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.txt index cc443fcf4bec..d2250498c36d 100644 --- a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.txt +++ b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.txt @@ -23,7 +23,7 @@ For each opp entry in 'operating-points-v2' table: matches, the OPP gets enabled. Optional properties: -- core-supply: Phandle of voltage regulator of the SoC "core" power domain. +- power-domains: Phandle of the SoC "core" power domain. Child device nodes describe the memory settings for different configurations and clock rates. @@ -48,7 +48,7 @@ Example: interrupts = <0 78 0x04>; clocks = <_car TEGRA20_CLK_EMC>; nvidia,memory-controller = <>; - core-supply = <_vdd_reg>; + power-domains = <>; operating-points-v2 = <_table>; } -- 2.30.2
[PATCH v1 0/6] NVIDIA Tegra memory improvements
Hi, This series replaces the raw voltage regulator with a power domain that will be managing SoC core voltage. The core power domain patches are still under review, but it's clear at this point that this is the way we will implement the DVFS support. The remaining Tegra20 memory bindings are converted to schema. I also made a small improvement to the memory drivers. Dmitry Osipenko (6): dt-bindings: memory: tegra20: emc: Replace core regulator with power domain dt-bindings: memory: tegra30: emc: Replace core regulator with power domain dt-bindings: memory: tegra124: emc: Replace core regulator with power domain dt-bindings: memory: tegra20: mc: Convert to schema dt-bindings: memory: tegra20: emc: Convert to schema memory: tegra: Print out info-level once per driver probe .../nvidia,tegra124-emc.yaml | 7 +- .../memory-controllers/nvidia,tegra20-emc.txt | 130 .../nvidia,tegra20-emc.yaml | 294 ++ .../memory-controllers/nvidia,tegra20-mc.txt | 40 --- .../memory-controllers/nvidia,tegra20-mc.yaml | 78 + .../nvidia,tegra30-emc.yaml | 7 +- drivers/memory/tegra/tegra124-emc.c | 12 +- drivers/memory/tegra/tegra20-emc.c| 20 +- drivers/memory/tegra/tegra30-emc.c| 18 +- 9 files changed, 405 insertions(+), 201 deletions(-) delete mode 100644 Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.txt create mode 100644 Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.yaml delete mode 100644 Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-mc.txt create mode 100644 Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-mc.yaml -- 2.30.2
Re: [PATCH v1] i2c: tegra: Improve handling of i2c_recover_bus()
29.03.2021 22:15, Wolfram Sang пишет: > On Mon, Mar 29, 2021 at 10:05:46PM +0300, Dmitry Osipenko wrote: >> The i2c_recover_bus() returns -EOPNOTSUPP if bus recovery isn't wired up, >> which the case for older Tegra SoCs at the moment. This error code is then >> propagated to I2C client and might be confusing, thus return -EIO instead. > > Hmm, makes sense. Maybe we should change it in the core? But with EBUSY > instead? > This also should be a good variant. I'll update the patch, thanks.
[PATCH v1] i2c: tegra: Improve handling of i2c_recover_bus()
The i2c_recover_bus() returns -EOPNOTSUPP if bus recovery isn't wired up,
which the case for older Tegra SoCs at the moment. This error code is then
propagated to I2C client and might be confusing, thus return -EIO instead.
Signed-off-by: Dmitry Osipenko
---
drivers/i2c/busses/i2c-tegra.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index c883044715f3..cb5e3cc96160 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -1196,8 +1196,14 @@ static int tegra_i2c_error_recover(struct tegra_i2c_dev
*i2c_dev,
/* start recovery upon arbitration loss in single master mode */
if (i2c_dev->msg_err == I2C_ERR_ARBITRATION_LOST) {
- if (!i2c_dev->multimaster_mode)
- return i2c_recover_bus(_dev->adapter);
+ if (!i2c_dev->multimaster_mode) {
+ int err = i2c_recover_bus(_dev->adapter);
+
+ if (err == -EOPNOTSUPP)
+ return -EIO;
+
+ return err;
+ }
return -EAGAIN;
}
--
2.30.2
[PATCH v1] Input: elants_i2c - drop zero-checking of ABS_MT_TOUCH_MAJOR resolution
Drop unnecessary zero-checking of ABS_MT_TOUCH_MAJOR resolution since there is no difference between setting resolution to 0 vs not setting it at all. This change makes code cleaner a tad. Suggested-by: Dmitry Torokhov Signed-off-by: Dmitry Osipenko --- drivers/input/touchscreen/elants_i2c.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/input/touchscreen/elants_i2c.c b/drivers/input/touchscreen/elants_i2c.c index 4c2b579f6c8b..7ddc24471819 100644 --- a/drivers/input/touchscreen/elants_i2c.c +++ b/drivers/input/touchscreen/elants_i2c.c @@ -1449,8 +1449,7 @@ static int elants_i2c_probe(struct i2c_client *client, input_abs_set_res(ts->input, ABS_MT_POSITION_X, ts->x_res); input_abs_set_res(ts->input, ABS_MT_POSITION_Y, ts->y_res); - if (ts->major_res > 0) - input_abs_set_res(ts->input, ABS_MT_TOUCH_MAJOR, ts->major_res); + input_abs_set_res(ts->input, ABS_MT_TOUCH_MAJOR, ts->major_res); error = input_mt_init_slots(ts->input, MAX_CONTACT_NUM, INPUT_MT_DIRECT | INPUT_MT_DROP_UNUSED); -- 2.30.2
[PATCH v1 2/2] iommu/tegra-smmu: Revert workaround that was needed for Nyan Big Chromebook
The previous commit fixes problem where display client was attaching too
early to IOMMU during kernel boot in a multi-platform kernel configuration
which enables CONFIG_ARM_DMA_USE_IOMMU=y. The workaround that helped to
defer the IOMMU attachment for Nyan Big Chromebook isn't needed anymore,
revert it.
Signed-off-by: Dmitry Osipenko
---
drivers/iommu/tegra-smmu.c | 71 +-
1 file changed, 1 insertion(+), 70 deletions(-)
diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index af1e4b5adb27..572a4544ae88 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -869,69 +869,10 @@ static phys_addr_t tegra_smmu_iova_to_phys(struct
iommu_domain *domain,
return SMMU_PFN_PHYS(pfn) + SMMU_OFFSET_IN_PAGE(iova);
}
-static struct tegra_smmu *tegra_smmu_find(struct device_node *np)
-{
- struct platform_device *pdev;
- struct tegra_mc *mc;
-
- pdev = of_find_device_by_node(np);
- if (!pdev)
- return NULL;
-
- mc = platform_get_drvdata(pdev);
- if (!mc)
- return NULL;
-
- return mc->smmu;
-}
-
-static int tegra_smmu_configure(struct tegra_smmu *smmu, struct device *dev,
- struct of_phandle_args *args)
-{
- const struct iommu_ops *ops = smmu->iommu.ops;
- int err;
-
- err = iommu_fwspec_init(dev, >of_node->fwnode, ops);
- if (err < 0) {
- dev_err(dev, "failed to initialize fwspec: %d\n", err);
- return err;
- }
-
- err = ops->of_xlate(dev, args);
- if (err < 0) {
- dev_err(dev, "failed to parse SW group ID: %d\n", err);
- iommu_fwspec_free(dev);
- return err;
- }
-
- return 0;
-}
-
static struct iommu_device *tegra_smmu_probe_device(struct device *dev)
{
- struct device_node *np = dev->of_node;
- struct tegra_smmu *smmu = NULL;
- struct of_phandle_args args;
- unsigned int index = 0;
- int err;
-
- while (of_parse_phandle_with_args(np, "iommus", "#iommu-cells", index,
- ) == 0) {
- smmu = tegra_smmu_find(args.np);
- if (smmu) {
- err = tegra_smmu_configure(smmu, dev, );
-
- if (err < 0) {
- of_node_put(args.np);
- return ERR_PTR(err);
- }
- }
-
- of_node_put(args.np);
- index++;
- }
+ struct tegra_smmu *smmu = dev_iommu_priv_get(dev);
- smmu = dev_iommu_priv_get(dev);
if (!smmu)
return ERR_PTR(-ENODEV);
@@ -1158,16 +1099,6 @@ struct tegra_smmu *tegra_smmu_probe(struct device *dev,
if (!smmu)
return ERR_PTR(-ENOMEM);
- /*
-* This is a bit of a hack. Ideally we'd want to simply return this
-* value. However the IOMMU registration process will attempt to add
-* all devices to the IOMMU when bus_set_iommu() is called. In order
-* not to rely on global variables to track the IOMMU instance, we
-* set it here so that it can be looked up from the .probe_device()
-* callback via the IOMMU device's .drvdata field.
-*/
- mc->smmu = smmu;
-
size = BITS_TO_LONGS(soc->num_asids) * sizeof(long);
smmu->asids = devm_kzalloc(dev, size, GFP_KERNEL);
--
2.30.2
[PATCH v1 1/2] iommu/tegra-smmu: Defer attachment of display clients
All consumer-grade Android and Chromebook devices show a splash screen
on boot and then display is left enabled when kernel is booted. This
behaviour is unacceptable in a case of implicit IOMMU domains to which
devices are attached during kernel boot since devices, like display
controller, may perform DMA at that time. We can work around this problem
by deferring the enable of SMMU translation for a specific devices,
like a display controller, until the first IOMMU mapping is created,
which works good enough in practice because by that time h/w is already
stopped.
Signed-off-by: Dmitry Osipenko
---
drivers/iommu/tegra-smmu.c | 71 ++
1 file changed, 71 insertions(+)
diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index 602aab98c079..af1e4b5adb27 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -60,6 +60,8 @@ struct tegra_smmu_as {
dma_addr_t pd_dma;
unsigned id;
u32 attr;
+ bool display_attached[2];
+ bool attached_devices_need_sync;
};
static struct tegra_smmu_as *to_smmu_as(struct iommu_domain *dom)
@@ -78,6 +80,10 @@ static inline u32 smmu_readl(struct tegra_smmu *smmu,
unsigned long offset)
return readl(smmu->regs + offset);
}
+/* all Tegra SoCs use the same group IDs for displays */
+#define SMMU_SWGROUP_DC1
+#define SMMU_SWGROUP_DCB 2
+
#define SMMU_CONFIG 0x010
#define SMMU_CONFIG_ENABLE (1 << 0)
@@ -253,6 +259,20 @@ static inline void smmu_flush(struct tegra_smmu *smmu)
smmu_readl(smmu, SMMU_PTB_ASID);
}
+static int smmu_swgroup_to_display_id(unsigned int swgroup)
+{
+ switch (swgroup) {
+ case SMMU_SWGROUP_DC:
+ return 0;
+
+ case SMMU_SWGROUP_DCB:
+ return 1;
+
+ default:
+ return -1;
+ }
+}
+
static int tegra_smmu_alloc_asid(struct tegra_smmu *smmu, unsigned int *idp)
{
unsigned long id;
@@ -318,6 +338,9 @@ static struct iommu_domain
*tegra_smmu_domain_alloc(unsigned type)
as->domain.geometry.aperture_end = 0x;
as->domain.geometry.force_aperture = true;
+ /* work around implicit attachment of devices with active DMA */
+ as->attached_devices_need_sync = true;
+
return >domain;
}
@@ -410,6 +433,31 @@ static void tegra_smmu_disable(struct tegra_smmu *smmu,
unsigned int swgroup,
}
}
+static void tegra_smmu_attach_deferred_devices(struct iommu_domain *domain)
+{
+ struct tegra_smmu_as *as = to_smmu_as(domain);
+
+ if (!as->attached_devices_need_sync)
+ return;
+
+ if (as->display_attached[0] || as->display_attached[1]) {
+ struct tegra_smmu *smmu = as->smmu;
+ unsigned int i;
+
+ for (i = 0; i < smmu->soc->num_clients; i++) {
+ const struct tegra_mc_client *client =
>soc->clients[i];
+ const int disp_id =
smmu_swgroup_to_display_id(client->swgroup);
+
+ if (disp_id < 0 || !as->display_attached[disp_id])
+ continue;
+
+ tegra_smmu_enable(smmu, client->swgroup, as->id);
+ }
+ }
+
+ as->attached_devices_need_sync = false;
+}
+
static int tegra_smmu_as_prepare(struct tegra_smmu *smmu,
struct tegra_smmu_as *as)
{
@@ -495,10 +543,26 @@ static int tegra_smmu_attach_dev(struct iommu_domain
*domain,
return -ENOENT;
for (index = 0; index < fwspec->num_ids; index++) {
+ const unsigned int swgroup = fwspec->ids[index];
+ const int disp_id = smmu_swgroup_to_display_id(swgroup);
+
err = tegra_smmu_as_prepare(smmu, as);
if (err)
goto disable;
+ if (disp_id >= 0) {
+ as->display_attached[disp_id] = true;
+
+ /*
+* In most cases display is performing DMA before
+* driver is initialized by showing a splash screen
+* and in this case we should defer the h/w attachment
+* until the first mapping is created by display driver.
+*/
+ if (as->attached_devices_need_sync)
+ continue;
+ }
+
tegra_smmu_enable(smmu, fwspec->ids[index], as->id);
}
@@ -527,6 +591,12 @@ static void tegra_smmu_detach_dev(struct iommu_domain
*domain, struct device *de
return;
for (index = 0; index < fwspec->num_ids; index++) {
+ const unsigned int swgroup = fwspec->ids[index];
+ const int disp_id = smmu_swgroup_to_display_id(swgroup)
Re: [PATCH v1] Input: elants_i2c - fix division by zero if firmware reports zero phys size
28.03.2021 07:44, Dmitry Torokhov пишет:
> Hi Dmitry,
>
> On Tue, Mar 02, 2021 at 01:08:24PM +0300, Dmitry Osipenko wrote:
>> Touchscreen firmware of ASUS Transformer TF700T reports zeros for the phys
>> size. Hence check whether the size is zero and don't set the resolution in
>> this case.
>>
>> Reported-by: Jasper Korten
>> Signed-off-by: Dmitry Osipenko
>> ---
>>
>> Please note that ASUS TF700T isn't yet supported by upstream kernel,
>> hence this is not a critical fix.
>>
>> drivers/input/touchscreen/elants_i2c.c | 8 +---
>> 1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/input/touchscreen/elants_i2c.c
>> b/drivers/input/touchscreen/elants_i2c.c
>> index 4c2b579f6c8b..a2e1cc4192b0 100644
>> --- a/drivers/input/touchscreen/elants_i2c.c
>> +++ b/drivers/input/touchscreen/elants_i2c.c
>> @@ -1441,14 +1441,16 @@ static int elants_i2c_probe(struct i2c_client
>> *client,
>>
>> touchscreen_parse_properties(ts->input, true, >prop);
>>
>> -if (ts->chip_id == EKTF3624) {
>> +if (ts->chip_id == EKTF3624 && ts->phy_x && ts->phy_y) {
>> /* calculate resolution from size */
>> ts->x_res = DIV_ROUND_CLOSEST(ts->prop.max_x, ts->phy_x);
>> ts->y_res = DIV_ROUND_CLOSEST(ts->prop.max_y, ts->phy_y);
>> }
>>
>> -input_abs_set_res(ts->input, ABS_MT_POSITION_X, ts->x_res);
>> -input_abs_set_res(ts->input, ABS_MT_POSITION_Y, ts->y_res);
>> +if (ts->x_res > 0)
>> +input_abs_set_res(ts->input, ABS_MT_POSITION_X, ts->x_res);
>
> There is absolutely no difference between setting respluton to 0 vs not
> setting it at all, so I dropped the conditionals and applied.
>
>> +if (ts->y_res > 0)
>> +input_abs_set_res(ts->input, ABS_MT_POSITION_Y, ts->y_res);
>> if (ts->major_res > 0)
>
> We could drop this condition as well.
>
>> input_abs_set_res(ts->input, ABS_MT_TOUCH_MAJOR, ts->major_res);
I'll make a follow up patch to clean up this condition, if you haven't
done it yet. Thanks!
Re: [PATCH] iommu/tegra-smmu: Fix mc errors on tegra124-nyan
28.03.2021 18:25, Dmitry Osipenko пишет: > 03.03.2021 12:47, Dmitry Osipenko пишет: >> 03.03.2021 02:08, Nicolin Chen пишет: >>> On Sat, Feb 27, 2021 at 12:59:17PM +0300, Dmitry Osipenko wrote: >>>> 25.02.2021 09:27, Nicolin Chen пишет: >>>> ... >>>>>> The partially revert should be okay, but it's not clear to me what makes >>>>>> difference for T124 since I don't see that problem on T30, which also >>>>>> has active display at a boot time. >>>>> Hmm..do you see ->attach_dev() is called from host1x_client_iommu_attach >>>>> or from of_dma_configure_id/arch_setup_dma_ops? >>>>> >>>> I applied yours debug-patch, please see dmesg.txt attached to the email. >>>> Seems probe-defer of the tegra-dc driver prevents the implicit >>>> tegra_smmu_attach_dev, so it happens to work by accident. >>>> [0.327826] tegra-dc 5420.dc: ---tegra_smmu_of_xlate: id 1 >>>> [0.328641] [] (tegra_smmu_of_xlate) from [] >>>> (of_iommu_xlate+0x51/0x70) >>>> [0.328740] [] (of_iommu_xlate) from [] >>>> (of_iommu_configure+0x127/0x150) >>>> [0.328896] [] (of_iommu_configure) from [] >>>> (of_dma_configure_id+0x1fb/0x2ec) >>>> [0.329060] [] (of_dma_configure_id) from [] >>>> (really_probe+0x7b/0x2a0) >>>> [0.331438] tegra-dc 5420.dc: tegra_smmu_probe_device, 822 >>>> [0.332234] [] (tegra_smmu_probe_device) from [] >>>> (__iommu_probe_device+0x35/0x1c4) >>>> [0.332391] [] (__iommu_probe_device) from [] >>>> (iommu_probe_device+0x19/0xec) >>>> [0.332545] [] (iommu_probe_device) from [] >>>> (of_iommu_configure+0xfb/0x150) >>>> [0.332701] [] (of_iommu_configure) from [] >>>> (of_dma_configure_id+0x1fb/0x2ec) >>>> [0.332804] [] (of_dma_configure_id) from [] >>>> (really_probe+0x7b/0x2a0) >>>> [0.335202] tegra-dc 5420.dc: -iommu_group_get_for_dev, 1572 >>>> [0.335292] tegra-dc 5420.dc: -tegra_smmu_device_group, 862 >>>> [0.335474] tegra-dc 5420.dc: -tegra_smmu_device_group, >>>> 909: 1: drm >>>> [0.335566] tegra-dc 5420.dc: -iommu_group_get_for_dev, 1574 >>>> [0.335718] tegra-dc 5420.dc: -iommu_group_add_device, 858 >>>> [0.335862] tegra-dc 5420.dc: Adding to iommu group 1 >>>> [0.335955] tegra-dc 5420.dc: -iommu_alloc_default_domain, >>>> 1543: type 3 >>>> [0.336101] iommu: --iommu_group_alloc_default_domain: platform, >>>> (null), drm >>>> [0.336187] -tegra_smmu_domain_alloc, 284: type 3 >>> [0.336968] [] (tegra_smmu_domain_alloc) from [] >>> (iommu_group_alloc_default_domain+0x4b/0xfa) >>>> [0.337127] [] (iommu_group_alloc_default_domain) from >>>> [] (iommu_probe_device+0x69/0xec) >>>> [0.337285] [] (iommu_probe_device) from [] >>>> (of_iommu_configure+0xfb/0x150) >>>> [0.337441] [] (of_iommu_configure) from [] >>>> (of_dma_configure_id+0x1fb/0x2ec) >>>> [0.337599] [] (of_dma_configure_id) from [] >>>> (really_probe+0x7b/0x2a0) >>>> [0.339913] tegra-dc 5420.dc: -iommu_probe_device, 272 >>>> [0.348144] tegra-dc 5420.dc: failed to probe RGB output: -517 >>> Hmm..not sure where this EPROBE_DEFER comes from. >> DC driver on Nexus 7 depends on LVDS bridge and display panel, which >> cause the probe defer. >> >>> But you are right, >>> as of_dma_configure_id() returns because of that so it didn't run to >>> arch_setup_dma_ops() call, which allocates an UNMANAGED iommu domain >>> and attaches DC to it on Tegra124. >>> >>> By the way, anyone can accept this change? It doesn't feel right to >>> leave a regression in the newer release... > > Guys, I have a good and bad news. > > The good news is that I figured out why I didn't see this problem on > Nexus 7 and the reason is that I had CONFIG_ARM_DMA_USE_IOMMU=n. > > The other good news is that I have a simple workaround which fixes the > implicit IOMMU problem by deferring the ASID enabling for display clients. > > The bad news is that CONFIG_ARM_DMA_USE_IOMMU=y breaks GPU (DRM, host1x) > drivers because they aren't properly prepared to this case and > CONFIG_ARM_DMA_USE_IOMMU is enabled in multi-platform kernel config. I > will try to fix up the drivers, but not sure how much time this may take. > Oh, actually the old workaround with arm_iommu_detach_device() still works, so we just need to bring it back. I'll prepare the patches.
Re: [PATCH] iommu/tegra-smmu: Fix mc errors on tegra124-nyan
03.03.2021 12:47, Dmitry Osipenko пишет: > 03.03.2021 02:08, Nicolin Chen пишет: >> On Sat, Feb 27, 2021 at 12:59:17PM +0300, Dmitry Osipenko wrote: >>> 25.02.2021 09:27, Nicolin Chen пишет: >>> ... >>>>> The partially revert should be okay, but it's not clear to me what makes >>>>> difference for T124 since I don't see that problem on T30, which also >>>>> has active display at a boot time. >>>> Hmm..do you see ->attach_dev() is called from host1x_client_iommu_attach >>>> or from of_dma_configure_id/arch_setup_dma_ops? >>>> >>> I applied yours debug-patch, please see dmesg.txt attached to the email. >>> Seems probe-defer of the tegra-dc driver prevents the implicit >>> tegra_smmu_attach_dev, so it happens to work by accident. >>> [0.327826] tegra-dc 5420.dc: ---tegra_smmu_of_xlate: id 1 >>> [0.328641] [] (tegra_smmu_of_xlate) from [] >>> (of_iommu_xlate+0x51/0x70) >>> [0.328740] [] (of_iommu_xlate) from [] >>> (of_iommu_configure+0x127/0x150) >>> [0.328896] [] (of_iommu_configure) from [] >>> (of_dma_configure_id+0x1fb/0x2ec) >>> [0.329060] [] (of_dma_configure_id) from [] >>> (really_probe+0x7b/0x2a0) >>> [0.331438] tegra-dc 5420.dc: tegra_smmu_probe_device, 822 >>> [0.332234] [] (tegra_smmu_probe_device) from [] >>> (__iommu_probe_device+0x35/0x1c4) >>> [0.332391] [] (__iommu_probe_device) from [] >>> (iommu_probe_device+0x19/0xec) >>> [0.332545] [] (iommu_probe_device) from [] >>> (of_iommu_configure+0xfb/0x150) >>> [0.332701] [] (of_iommu_configure) from [] >>> (of_dma_configure_id+0x1fb/0x2ec) >>> [0.332804] [] (of_dma_configure_id) from [] >>> (really_probe+0x7b/0x2a0) >>> [0.335202] tegra-dc 5420.dc: -iommu_group_get_for_dev, 1572 >>> [0.335292] tegra-dc 5420.dc: -tegra_smmu_device_group, 862 >>> [0.335474] tegra-dc 5420.dc: -tegra_smmu_device_group, 909: >>> 1: drm >>> [0.335566] tegra-dc 5420.dc: -iommu_group_get_for_dev, 1574 >>> [0.335718] tegra-dc 5420.dc: -iommu_group_add_device, 858 >>> [0.335862] tegra-dc 5420.dc: Adding to iommu group 1 >>> [0.335955] tegra-dc 5420.dc: -iommu_alloc_default_domain, >>> 1543: type 3 >>> [0.336101] iommu: --iommu_group_alloc_default_domain: platform, >>> (null), drm >>> [0.336187] -tegra_smmu_domain_alloc, 284: type 3 >> [0.336968] [] (tegra_smmu_domain_alloc) from [] >> (iommu_group_alloc_default_domain+0x4b/0xfa) >>> [0.337127] [] (iommu_group_alloc_default_domain) from >>> [] (iommu_probe_device+0x69/0xec) >>> [0.337285] [] (iommu_probe_device) from [] >>> (of_iommu_configure+0xfb/0x150) >>> [0.337441] [] (of_iommu_configure) from [] >>> (of_dma_configure_id+0x1fb/0x2ec) >>> [0.337599] [] (of_dma_configure_id) from [] >>> (really_probe+0x7b/0x2a0) >>> [0.339913] tegra-dc 5420.dc: -iommu_probe_device, 272 >>> [0.348144] tegra-dc 5420.dc: failed to probe RGB output: -517 >> Hmm..not sure where this EPROBE_DEFER comes from. > DC driver on Nexus 7 depends on LVDS bridge and display panel, which > cause the probe defer. > >> But you are right, >> as of_dma_configure_id() returns because of that so it didn't run to >> arch_setup_dma_ops() call, which allocates an UNMANAGED iommu domain >> and attaches DC to it on Tegra124. >> >> By the way, anyone can accept this change? It doesn't feel right to >> leave a regression in the newer release... Guys, I have a good and bad news. The good news is that I figured out why I didn't see this problem on Nexus 7 and the reason is that I had CONFIG_ARM_DMA_USE_IOMMU=n. The other good news is that I have a simple workaround which fixes the implicit IOMMU problem by deferring the ASID enabling for display clients. The bad news is that CONFIG_ARM_DMA_USE_IOMMU=y breaks GPU (DRM, host1x) drivers because they aren't properly prepared to this case and CONFIG_ARM_DMA_USE_IOMMU is enabled in multi-platform kernel config. I will try to fix up the drivers, but not sure how much time this may take.
Re: [PATCH v1] Input: elants_i2c - fix division by zero if firmware reports zero phys size
02.03.2021 13:08, Dmitry Osipenko пишет:
> Touchscreen firmware of ASUS Transformer TF700T reports zeros for the phys
> size. Hence check whether the size is zero and don't set the resolution in
> this case.
>
> Reported-by: Jasper Korten
> Signed-off-by: Dmitry Osipenko
> ---
>
> Please note that ASUS TF700T isn't yet supported by upstream kernel,
> hence this is not a critical fix.
>
> drivers/input/touchscreen/elants_i2c.c | 8 +---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/input/touchscreen/elants_i2c.c
> b/drivers/input/touchscreen/elants_i2c.c
> index 4c2b579f6c8b..a2e1cc4192b0 100644
> --- a/drivers/input/touchscreen/elants_i2c.c
> +++ b/drivers/input/touchscreen/elants_i2c.c
> @@ -1441,14 +1441,16 @@ static int elants_i2c_probe(struct i2c_client *client,
>
> touchscreen_parse_properties(ts->input, true, >prop);
>
> - if (ts->chip_id == EKTF3624) {
> + if (ts->chip_id == EKTF3624 && ts->phy_x && ts->phy_y) {
> /* calculate resolution from size */
> ts->x_res = DIV_ROUND_CLOSEST(ts->prop.max_x, ts->phy_x);
> ts->y_res = DIV_ROUND_CLOSEST(ts->prop.max_y, ts->phy_y);
> }
>
> - input_abs_set_res(ts->input, ABS_MT_POSITION_X, ts->x_res);
> - input_abs_set_res(ts->input, ABS_MT_POSITION_Y, ts->y_res);
> + if (ts->x_res > 0)
> + input_abs_set_res(ts->input, ABS_MT_POSITION_X, ts->x_res);
> + if (ts->y_res > 0)
> + input_abs_set_res(ts->input, ABS_MT_POSITION_Y, ts->y_res);
> if (ts->major_res > 0)
> input_abs_set_res(ts->input, ABS_MT_TOUCH_MAJOR, ts->major_res);
>
>
Hi,
This is a very minor fix, but still will be nice if we could get it into
5.13 in order to have one less patch to care about. Thanks in advance!
Re: [PATCH v1 1/2] memory: tegra20: Correct comment to MC_STAT registers writes
26.03.2021 10:38, Krzysztof Kozlowski пишет: > On 25/03/2021 18:20, Dmitry Osipenko wrote: >> 24.03.2021 00:04, Dmitry Osipenko пишет: >>> The code was changed multiple times and the comment to MC_STAT >>> registers writes became slightly outdated. The MC_STAT programming >>> now isn't hardcoded to the "bandwidth" mode, let's clarify this in >>> the comment. >>> >>> Signed-off-by: Dmitry Osipenko >>> --- >>> drivers/memory/tegra/tegra20.c | 5 ++--- >>> 1 file changed, 2 insertions(+), 3 deletions(-) >>> >>> diff --git a/drivers/memory/tegra/tegra20.c b/drivers/memory/tegra/tegra20.c >>> index 14caf5b9324c..4659c0cea30d 100644 >>> --- a/drivers/memory/tegra/tegra20.c >>> +++ b/drivers/memory/tegra/tegra20.c >>> @@ -451,9 +451,8 @@ static void tegra20_mc_stat_gather(struct >>> tegra20_mc_stat *stat) >>> control_1 = tegra20_mc_stat_gather_control(>gather1); >>> >>> /* >>> -* Reset statistic gathers state, select bandwidth mode for the >>> -* statistics collection mode and set clocks counter saturation >>> -* limit to maximum. >>> +* Reset statistic gathers state, select statistics collection mode >>> +* and set clocks counter saturation limit to maximum. >>> */ >>> mc_writel(mc, 0x, MC_STAT_CONTROL); >>> mc_writel(mc, control_0, MC_STAT_EMC_CONTROL_0); >>> >> >> Krzysztof, please feel free to squash these 2 minor follow up patches >> into the original patch which added the the debug support, if you prefer >> this way more. I happened to notice these small itches only after you >> already picked up the previous patch. > > No problem, in general I prefer to have incremental improvements. Thank you.
Re: [PATCH v4 3/6] dt-bindings: power: tegra: Add binding for core power domain
25.03.2021 17:49, Thierry Reding пишет:
> On Wed, Mar 24, 2021 at 02:01:29AM +0300, Dmitry Osipenko wrote:
>> 24.03.2021 01:48, Rob Herring пишет:
>>> On Sun, Mar 14, 2021 at 07:48:07PM +0300, Dmitry Osipenko wrote:
>>>> All NVIDIA Tegra SoCs have a core power domain where majority of hardware
>>>> blocks reside. Add binding for the core power domain.
>>>>
>>>> Signed-off-by: Dmitry Osipenko
>>>> ---
>>>> .../power/nvidia,tegra20-core-domain.yaml | 51 +++
>>>> 1 file changed, 51 insertions(+)
>>>> create mode 100644
>>>> Documentation/devicetree/bindings/power/nvidia,tegra20-core-domain.yaml
>>>>
>>>> diff --git
>>>> a/Documentation/devicetree/bindings/power/nvidia,tegra20-core-domain.yaml
>>>> b/Documentation/devicetree/bindings/power/nvidia,tegra20-core-domain.yaml
>>>> new file mode 100644
>>>> index ..4692489d780a
>>>> --- /dev/null
>>>> +++
>>>> b/Documentation/devicetree/bindings/power/nvidia,tegra20-core-domain.yaml
>>>> @@ -0,0 +1,51 @@
>>>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: http://devicetree.org/schemas/power/nvidia,tegra20-core-domain.yaml#
>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>> +
>>>> +title: NVIDIA Tegra Core Power Domain
>>>> +
>>>> +maintainers:
>>>> + - Dmitry Osipenko
>>>> + - Jon Hunter
>>>> + - Thierry Reding
>>>> +
>>>> +allOf:
>>>> + - $ref: power-domain.yaml#
>>>> +
>>>> +properties:
>>>> + compatible:
>>>> +enum:
>>>> + - nvidia,tegra20-core-domain
>>>> + - nvidia,tegra30-core-domain
>>>> +
>>>> + operating-points-v2:
>>>> +description:
>>>> + Should contain level, voltages and opp-supported-hw property.
>>>> + The supported-hw is a bitfield indicating SoC speedo or process
>>>> + ID mask.
>>>> +
>>>> + "#power-domain-cells":
>>>> +const: 0
>>>> +
>>>> + power-supply:
>>>> +description:
>>>> + Phandle to voltage regulator connected to the SoC Core power rail.
>>>> +
>>>> +required:
>>>> + - compatible
>>>> + - operating-points-v2
>>>> + - "#power-domain-cells"
>>>> + - power-supply
>>>> +
>>>> +additionalProperties: false
>>>> +
>>>> +examples:
>>>> + - |
>>>> +power-domain {
>>>> +compatible = "nvidia,tegra20-core-domain";
>>>> +operating-points-v2 = <_table>;
>>>> +power-supply = <>;
>>>> +#power-domain-cells = <0>;
>>>
>>> AFAICT, there's no way to access this 'hardware'?
>> correct
>
> To avoid exposing this "virtual" device in device tree, could this
> instead be modelled as a child node of the PMC node? We already expose a
> couple of generic power domains that way on Tegra210 and later, so
> perhaps some of that infrastructure can be reused? I suppose given that
> this is different from the standard powergate domains that we expose so
> far, this may need a different implementation, but from a device tree
> bindings point of view it could fit in with that.
At a quick glance this should be too troublesome because OPP and regulator
frameworks require a proper/real backing device.
Perhaps we could either turn the whole PMC into a core-domain or add a virtual
device as a child of PMC, like this:
diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
index 79364cdafeab..717273048caf 100644
--- a/arch/arm/boot/dts/tegra20.dtsi
+++ b/arch/arm/boot/dts/tegra20.dtsi
@@ -850,6 +850,12 @@ pd_mpe: mpe {
#power-domain-cells = <0>;
};
};
+
+ pd_core: core-domain {
+ compatible = "nvidia,tegra20-core-domain";
+ operating-points-v2 = <_opp_table>;
+ #power-domain-cells = <0>;
+ };
};
mc: memory-controller@7000f000 {
but then this is still a virtual device, although in a bit nicer way.
It feels like yours suggestion might result in a hardware description that is
closer to reality since PMC controls fan out of all power rails within SoC.
Re: [PATCH v1 1/2] memory: tegra20: Correct comment to MC_STAT registers writes
24.03.2021 00:04, Dmitry Osipenko пишет: > The code was changed multiple times and the comment to MC_STAT > registers writes became slightly outdated. The MC_STAT programming > now isn't hardcoded to the "bandwidth" mode, let's clarify this in > the comment. > > Signed-off-by: Dmitry Osipenko > --- > drivers/memory/tegra/tegra20.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/memory/tegra/tegra20.c b/drivers/memory/tegra/tegra20.c > index 14caf5b9324c..4659c0cea30d 100644 > --- a/drivers/memory/tegra/tegra20.c > +++ b/drivers/memory/tegra/tegra20.c > @@ -451,9 +451,8 @@ static void tegra20_mc_stat_gather(struct tegra20_mc_stat > *stat) > control_1 = tegra20_mc_stat_gather_control(>gather1); > > /* > - * Reset statistic gathers state, select bandwidth mode for the > - * statistics collection mode and set clocks counter saturation > - * limit to maximum. > + * Reset statistic gathers state, select statistics collection mode > + * and set clocks counter saturation limit to maximum. >*/ > mc_writel(mc, 0x, MC_STAT_CONTROL); > mc_writel(mc, control_0, MC_STAT_EMC_CONTROL_0); > Krzysztof, please feel free to squash these 2 minor follow up patches into the original patch which added the the debug support, if you prefer this way more. I happened to notice these small itches only after you already picked up the previous patch.
Re: [PATCH v4 3/5] soc/tegra: pmc: Ensure that clock rates aren't too high
25.03.2021 18:02, Dmitry Osipenko пишет:
> 25.03.2021 17:39, Thierry Reding пишет:
>> On Tue, Mar 02, 2021 at 03:25:00PM +0300, Dmitry Osipenko wrote:
>>> Switch all clocks of a power domain to a safe rate which is suitable
>>> for all possible voltages in order to ensure that hardware constraints
>>> aren't violated when power domain state toggles.
>>>
>>> Tested-by: Peter Geis # Ouya T30
>>> Tested-by: Nicolas Chauvet # PAZ00 T20 and TK1 T124
>>> Tested-by: Matt Merhar # Ouya T30
>>> Signed-off-by: Dmitry Osipenko
>>> ---
>>> drivers/soc/tegra/pmc.c | 92 -
>>> 1 file changed, 90 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
>>> index f970b615ee27..a87645fac735 100644
>>> --- a/drivers/soc/tegra/pmc.c
>>> +++ b/drivers/soc/tegra/pmc.c
>>> @@ -237,6 +237,7 @@ struct tegra_powergate {
>>> unsigned int id;
>>> struct clk **clks;
>>> unsigned int num_clks;
>>> + unsigned long *clk_rates;
>>> struct reset_control *reset;
>>> };
>>>
>>> @@ -641,6 +642,57 @@ static int __tegra_powergate_remove_clamping(struct
>>> tegra_pmc *pmc,
>>> return 0;
>>> }
>>>
>>> +static int tegra_powergate_prepare_clocks(struct tegra_powergate *pg)
>>> +{
>>> + unsigned long safe_rate = 100 * 1000 * 1000;
>>
>> This seems a bit arbitrary. Where did you come up with that value?
>>
>> I'm going to apply this to see how it fares in our testing.
>
> This value is chosen based on the OPP table voltages and frequencies.
>
> Maybe you could add this clarifying comment to the code(?):
>
> "There is no hardware unit on any of Tegra SoCs which requires higher
> voltages for the rates above 100MHz."
I meant below, of course :)
Re: [PATCH v4 3/5] soc/tegra: pmc: Ensure that clock rates aren't too high
25.03.2021 17:39, Thierry Reding пишет:
> On Tue, Mar 02, 2021 at 03:25:00PM +0300, Dmitry Osipenko wrote:
>> Switch all clocks of a power domain to a safe rate which is suitable
>> for all possible voltages in order to ensure that hardware constraints
>> aren't violated when power domain state toggles.
>>
>> Tested-by: Peter Geis # Ouya T30
>> Tested-by: Nicolas Chauvet # PAZ00 T20 and TK1 T124
>> Tested-by: Matt Merhar # Ouya T30
>> Signed-off-by: Dmitry Osipenko
>> ---
>> drivers/soc/tegra/pmc.c | 92 -
>> 1 file changed, 90 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
>> index f970b615ee27..a87645fac735 100644
>> --- a/drivers/soc/tegra/pmc.c
>> +++ b/drivers/soc/tegra/pmc.c
>> @@ -237,6 +237,7 @@ struct tegra_powergate {
>> unsigned int id;
>> struct clk **clks;
>> unsigned int num_clks;
>> +unsigned long *clk_rates;
>> struct reset_control *reset;
>> };
>>
>> @@ -641,6 +642,57 @@ static int __tegra_powergate_remove_clamping(struct
>> tegra_pmc *pmc,
>> return 0;
>> }
>>
>> +static int tegra_powergate_prepare_clocks(struct tegra_powergate *pg)
>> +{
>> +unsigned long safe_rate = 100 * 1000 * 1000;
>
> This seems a bit arbitrary. Where did you come up with that value?
>
> I'm going to apply this to see how it fares in our testing.
This value is chosen based on the OPP table voltages and frequencies.
Maybe you could add this clarifying comment to the code(?):
"There is no hardware unit on any of Tegra SoCs which requires higher
voltages for the rates above 100MHz."
Re: [PATCH v4 5/5] soc/tegra: pmc: Rate-limit error message about failed to acquire of reset
25.03.2021 17:42, Thierry Reding пишет:
> On Tue, Mar 02, 2021 at 03:25:02PM +0300, Dmitry Osipenko wrote:
>> PMC domain could be easily bombarded with the enable requests if there is
>> a problem in regards to acquiring reset control of a domain and kernel
>> log will be flooded with the error message in this case. Hence rate-limit
>> the message in order to prevent missing other important messages.
>>
>> Signed-off-by: Dmitry Osipenko
>> ---
>> drivers/soc/tegra/pmc.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
>> index bf29ea22480a..84ab27d85d92 100644
>> --- a/drivers/soc/tegra/pmc.c
>> +++ b/drivers/soc/tegra/pmc.c
>> @@ -868,8 +868,8 @@ static int tegra_genpd_power_off(struct
>> generic_pm_domain *domain)
>>
>> err = reset_control_acquire(pg->reset);
>> if (err < 0) {
>> -dev_err(dev, "failed to acquire resets for PM domain %s: %d\n",
>> -pg->genpd.name, err);
>> +dev_err_ratelimited(dev, "failed to acquire resets for PM
>> domain %s: %d\n",
>> +pg->genpd.name, err);
>
> That doesn't look right. This is a serious error condition that
> shouldn't happen at all. Ever. If this shows up even once we've got a
> serious bug somewhere and we need to fix it rather than "downplay" it
> by ratelimiting these errors.
>
> What's the exact use-case where you see this?
This was firing up badly while I was wiring up power management and
GENPD support to the GR3D and VDE drivers and testing them because of
the bugs that I was creating.
Looking back again at this now, I agree that the commit message isn't
good and could be improved. What about this variant:
"Rate-limit error message about failing to acquire reset in order to
prevent missing other important messages. This was proven to be very
useful to have for development and debugging purposes of a power
management support for various Tegra drivers".
Re: [PATCH v7] mm: cma: support sysfs
25.03.2021 01:23, John Hubbard пишет:
> On 3/24/21 3:11 PM, Dmitry Osipenko wrote:
>> 25.03.2021 01:01, John Hubbard пишет:
>>> On 3/24/21 2:31 PM, Dmitry Osipenko wrote:
>>>> ...
>>>>> +#include
>>>>> +
>>>>> +struct cma_kobject {
>>>>> + struct cma *cma;
>>>>> + struct kobject kobj;
>>>>
>>>> If you'll place the kobj as the first member of the struct, then
>>>> container_of will be a no-op.
>>>>
>>>
>>> However, *this does not matter*. Let's not get carried away. If
>>> container_of() ends up as a compile-time addition of +8, instead
>>> of +0, there is not going to be a visible effect in the world.
>>> Or do you have some perf data to the contrary?
>>>
>>> Sometimes these kinds of things matter. But other times, they are
>>> just pointless to fret about, and this is once such case.
>>
>> Performance is out of question here, my main point is about maintaining
>
> In that case, there is even less reason to harass people about the order
> of members of a struct.
>
>> a good coding style. Otherwise there is no point in not embedding kobj
>> into cma struct as well, IMO.
>
>
> We really don't need to worry about the order of members in a struct,
> from a "coding style" point of view. It is a solid step too far.
>
> Sorry if that sounds a little too direct. But this review has tended to
> go quite too far into nitpicks that are normally left as-is, and I've
> merely picked one that is particularly questionable. I realize that other
> coding communities have their own standards. Here, I'm explaining what
> I have observed about linux-mm and linux-kernel, which needs to be
> respected.
I tried to help as much as I could, sorry if this felt annoying to you
or anyone else.
I assume that linux-mm maintainers, like any other maintainers, should
skip all suggestions that are deemed as inappropriate to them.
Re: [PATCH v7] mm: cma: support sysfs
25.03.2021 00:55, Minchan Kim пишет: >> The tags are incorrect, I haven't suggested this change. > During the development, you have suggested many things > to make it clean. That suggested-by couldn't represent > all the detail but wanted to give credit for you, too > since you spent the time to make it better. > > Okay, since you didn't like it, I will remove it. > That's what the r-b tag is for, the suggested-by is about suggesting the patch in general. Looking forward to the final v8, thank you for yours efforts!
Re: [PATCH v7] mm: cma: support sysfs
25.03.2021 00:55, Minchan Kim пишет:
>>> +static ssize_t alloc_pages_success_show(struct kobject *kobj,
>>> + struct kobj_attribute *attr, char *buf)
>>> +{
>>> + struct cma *cma = cma_from_kobj(kobj);
>>> +
>>> + return sysfs_emit(buf, "%llu\n",
>>> + atomic64_read(>nr_pages_succeeded));
>> nit: Algnment isn't right, should be better to write it as single line, IMO.
> Let me make it align rather than single line since I know someone
> who keep asking us to not overflow 80 columns unless it's special.
>
I'm actually one of those guys who complains about 80 chars (where
necessary), but in this particular case it only hurts readability of the
code, IMO.
Re: [PATCH v7] mm: cma: support sysfs
25.03.2021 01:10, Dmitry Osipenko пишет: > 25.03.2021 00:55, Minchan Kim пишет: >>> There are no dereferences fixed by this patch. >> Let me add this: >> https://lore.kernel.org/linux-mm/20210316100433.17665-1-colin.k...@canonical.com/ >> > > The tag is invalid now, since you squashed the fix. I think you may add > "Co-developed-by: Colin Ian King ", but this > also should require to add the s-b from Colin, if he doesn't mind. > Ah, I now see that the s-b tag is already there.
Re: [PATCH v7] mm: cma: support sysfs
25.03.2021 01:01, John Hubbard пишет:
> On 3/24/21 2:31 PM, Dmitry Osipenko wrote:
>> ...
>>> +#include
>>> +
>>> +struct cma_kobject {
>>> + struct cma *cma;
>>> + struct kobject kobj;
>>
>> If you'll place the kobj as the first member of the struct, then
>> container_of will be a no-op.
>>
>
> However, *this does not matter*. Let's not get carried away. If
> container_of() ends up as a compile-time addition of +8, instead
> of +0, there is not going to be a visible effect in the world.
> Or do you have some perf data to the contrary?
>
> Sometimes these kinds of things matter. But other times, they are
> just pointless to fret about, and this is once such case.
Performance is out of question here, my main point is about maintaining
a good coding style. Otherwise there is no point in not embedding kobj
into cma struct as well, IMO.
Re: [PATCH v7] mm: cma: support sysfs
25.03.2021 00:55, Minchan Kim пишет: >> There are no dereferences fixed by this patch. > Let me add this: > https://lore.kernel.org/linux-mm/20210316100433.17665-1-colin.k...@canonical.com/ > The tag is invalid now, since you squashed the fix. I think you may add "Co-developed-by: Colin Ian King ", but this also should require to add the s-b from Colin, if he doesn't mind.
Re: [PATCH v7] mm: cma: support sysfs
25.03.2021 00:31, Dmitry Osipenko пишет: >> Reported-by: Dmitry Osipenko >> Tested-by: Dmitry Osipenko >> Suggested-by: Dmitry Osipenko > The tags are incorrect, I haven't suggested this change. The reported-by also should be removed.
Re: [PATCH v7] mm: cma: support sysfs
24.03.2021 23:55, Minchan Kim пишет:
> Since CMA is getting used more widely, it's more important to
> keep monitoring CMA statistics for system health since it's
> directly related to user experience.
>
> This patch introduces sysfs statistics for CMA, in order to provide
> some basic monitoring of the CMA allocator.
>
> * the number of CMA page successful allocations
> * the number of CMA page allocation failures
>
> These two values allow the user to calcuate the allocation
> failure rate for each CMA area.
>
> e.g.)
> /sys/kernel/mm/cma/WIFI/alloc_pages_[success|fail]
> /sys/kernel/mm/cma/SENSOR/alloc_pages_[success|fail]
> /sys/kernel/mm/cma/BLUETOOTH/alloc_pages_[success|fail]
>
> The cma_stat was intentionally allocated by dynamic allocation
> to harmonize with kobject lifetime management.
> https://lore.kernel.org/linux-mm/ycoamxqt6dzkc...@kroah.com/
>
> Reported-by: Dmitry Osipenko
> Tested-by: Dmitry Osipenko
> Suggested-by: Dmitry Osipenko
The tags are incorrect, I haven't suggested this change.
> Suggested-by: John Hubbard
> Suggested-by: Matthew Wilcox
> Reviewed-by: Greg Kroah-Hartman
> Reviewed-by: John Hubbard
> Addresses-Coverity: ("Dereference after null check")
There are no dereferences fixed by this patch.
> Signed-off-by: Colin Ian King
> Signed-off-by: Minchan Kim
> ---
...
> #include
> +#include
> +
> +struct cma_kobject {
> + struct cma *cma;
> + struct kobject kobj;
If you'll place the kobj as the first member of the struct, then
container_of will be a no-op.
...
> +#include
> +#include
> +#include
> +
> +#include "cma.h"
> +
> +void cma_sysfs_account_success_pages(struct cma *cma, unsigned long nr_pages)
> +{
> + atomic64_add(nr_pages, >nr_pages_succeeded);
> +}
> +
> +void cma_sysfs_account_fail_pages(struct cma *cma, unsigned long nr_pages)
> +{
> + atomic64_add(nr_pages, >nr_pages_failed);
> +}
> +
> +#define CMA_ATTR_RO(_name) \
> + static struct kobj_attribute _name##_attr = __ATTR_RO(_name)
nit: #defines and inlined helpers typically are placed at the top of the
code, after includes.
> +static inline struct cma *cma_from_kobj(struct kobject *kobj)
> +{
> + struct cma_kobject *cma_kobj = container_of(kobj, struct cma_kobject,
> + kobj);
> + struct cma *cma = cma_kobj->cma;
> +
> + return cma;
nit: you can write this as:
return container_of(kobj, struct cma_kobject, kobj)->cma;
> +}
> +
> +static ssize_t alloc_pages_success_show(struct kobject *kobj,
> + struct kobj_attribute *attr, char *buf)
> +{
> + struct cma *cma = cma_from_kobj(kobj);
> +
> + return sysfs_emit(buf, "%llu\n",
> + atomic64_read(>nr_pages_succeeded));
nit: Algnment isn't right, should be better to write it as single line, IMO.
...
> +static int __init cma_sysfs_init(void)
> +{
> + struct kobject *cma_kobj_root;
> + struct cma_kobject *cma_kobj;
> + struct cma *cma;
> + int i, err;
> +
> + cma_kobj_root = kobject_create_and_add("cma", mm_kobj);
> + if (!cma_kobj_root)
> + return -ENOMEM;
> +
> + for (i = 0; i < cma_area_count; i++) {
> + cma_kobj = kzalloc(sizeof(*cma_kobj), GFP_KERNEL);
> + if (!cma_kobj) {
> + err = -ENOMEM;
> + goto out;
> + }
> +
> + cma = _areas[i];
> + cma->cma_kobj = cma_kobj;
> + cma_kobj->cma = cma;
> + err = kobject_init_and_add(_kobj->kobj, _ktype,
> + cma_kobj_root, "%s", cma->name);
nit: Previousy algnment of the code was better here.
Otherwise this is okay to me:
Reviewed-by: Dmitry Osipenko
Tested-by: Dmitry Osipenko
Re: [PATCH] mm: cma: fix corruption cma_sysfs_alloc_pages_count
24.03.2021 22:57, Minchan Kim пишет:
> On Wed, Mar 24, 2021 at 10:49:58PM +0300, Dmitry Osipenko wrote:
>> 24.03.2021 22:43, Dmitry Osipenko пишет:
>>> 24.03.2021 22:20, Minchan Kim пишет:
>>>> static int __init cma_sysfs_init(void)
>>>> {
>>>> - int i = 0;
>>>> + struct kobject *cma_kobj_root;
>>>> + struct cma_kobject *cma_kobj;
>>>>struct cma *cma;
>>>> + unsigned int i;
>>>
>>>>while (--i >= 0) {
>>>
>>> Do you realize that this doesn't work anymore?
>>>
>>>>cma = _areas[i];
>>>> - kobject_put(>stat->kobj);
>>>> - }
>>>>
>>>> - kfree(cma_stats);
>>>> - kobject_put(cma_kobj);
>>>> + kobject_put(>cma_kobj->kobj);
>>>> + kfree(cma->cma_kobj);
>>>
>>> Freeing a null pointer?
>>>
>>>> + cma->cma_kobj = NULL;
>>>> + }
>>>> + kobject_put(cma_kobj_root);
>>>
>>
>> Please try to simulate the errors and check that error path is working
>> properly in the next version.
>>
>> Alternatively, we could remove the cma_kobj_release entirely, like Greg
>> suggested previously, and then don't care about cleaning up at all.
>
> Does he suggested it to remove cma_kobj_release?(Initially, I did but
> was rejected from Greg)
>
Alright, I haven't followed the previous threads fully and only saw the
reply where he suggested to removed it.
Re: [PATCH] mm: cma: fix corruption cma_sysfs_alloc_pages_count
24.03.2021 22:43, Dmitry Osipenko пишет:
> 24.03.2021 22:20, Minchan Kim пишет:
>> static int __init cma_sysfs_init(void)
>> {
>> -int i = 0;
>> +struct kobject *cma_kobj_root;
>> +struct cma_kobject *cma_kobj;
>> struct cma *cma;
>> +unsigned int i;
>
>> while (--i >= 0) {
>
> Do you realize that this doesn't work anymore?
>
>> cma = _areas[i];
>> -kobject_put(>stat->kobj);
>> -}
>>
>> -kfree(cma_stats);
>> -kobject_put(cma_kobj);
>> +kobject_put(>cma_kobj->kobj);
>> +kfree(cma->cma_kobj);
>
> Freeing a null pointer?
>
>> +cma->cma_kobj = NULL;
>> +}
>> +kobject_put(cma_kobj_root);
>
Please try to simulate the errors and check that error path is working
properly in the next version.
Alternatively, we could remove the cma_kobj_release entirely, like Greg
suggested previously, and then don't care about cleaning up at all.
Re: [PATCH] mm: cma: fix corruption cma_sysfs_alloc_pages_count
24.03.2021 22:20, Minchan Kim пишет:
> static int __init cma_sysfs_init(void)
> {
> - int i = 0;
> + struct kobject *cma_kobj_root;
> + struct cma_kobject *cma_kobj;
> struct cma *cma;
> + unsigned int i;
> while (--i >= 0) {
Do you realize that this doesn't work anymore?
> cma = _areas[i];
> - kobject_put(>stat->kobj);
> - }
>
> - kfree(cma_stats);
> - kobject_put(cma_kobj);
> + kobject_put(>cma_kobj->kobj);
> + kfree(cma->cma_kobj);
Freeing a null pointer?
> + cma->cma_kobj = NULL;
> + }
> + kobject_put(cma_kobj_root);
Re: [PATCH v3 09/14] ARM: tegra: acer-a500: Rename avdd to vdda of touchscreen node
24.03.2021 18:11, Thierry Reding пишет:
> On Tue, Mar 02, 2021 at 03:09:58PM +0300, Dmitry Osipenko wrote:
>> Rename avdd supply to vdda of the touchscreen node. The old supply name
>> was incorrect.
>>
>> Signed-off-by: Dmitry Osipenko
>> ---
>> arch/arm/boot/dts/tegra20-acer-a500-picasso.dts | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> This one didn't apply cleanly, but applying it manually was fine...
Great, thank you. This is the correct fix for this trouble.
>> diff --git a/arch/arm/boot/dts/tegra20-acer-a500-picasso.dts
>> b/arch/arm/boot/dts/tegra20-acer-a500-picasso.dts
>> index 8a98e4a9d994..d852527db707 100644
>> --- a/arch/arm/boot/dts/tegra20-acer-a500-picasso.dts
>> +++ b/arch/arm/boot/dts/tegra20-acer-a500-picasso.dts
>> @@ -449,7 +449,7 @@ touchscreen@4c {
>>
>> reset-gpios = < TEGRA_GPIO(Q, 7) GPIO_ACTIVE_LOW>;
>>
>> -avdd-supply = <_3v3_sys>;
>> +vdda-supply = <_3v3_sys>;
>> vdd-supply = <_3v3_sys>;
>>
>> atmel,wakeup-method = ;
>
> Looks like this line is not upstream. Did I miss a patch somewhere?
Indeed! I kept forgetting to rebase this patch properly, nevertheless
that line already got into -next a day ago [1].
[1]
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=f0a77ed9080a39a75faecff53fa37b3328926421
This [1] patch should be in yours inbox.
Re: [PATCH v6] mm: cma: support sysfs
24.03.2021 08:44, Minchan Kim пишет:
> On Tue, Mar 23, 2021 at 09:47:27PM -0700, John Hubbard wrote:
>> On 3/23/21 8:27 PM, Minchan Kim wrote:
>> ...
> +static int __init cma_sysfs_init(void)
> +{
> + unsigned int i;
> +
> + cma_kobj_root = kobject_create_and_add("cma", mm_kobj);
> + if (!cma_kobj_root)
> + return -ENOMEM;
> +
> + for (i = 0; i < cma_area_count; i++) {
> + int err;
> + struct cma *cma;
> + struct cma_kobject *cma_kobj;
> +
> + cma_kobj = kzalloc(sizeof(*cma_kobj), GFP_KERNEL);
> + if (!cma_kobj) {
> + kobject_put(cma_kobj_root);
> + return -ENOMEM;
This leaks little cma_kobj's all over the floor. :)
>>>
>>> I thought kobject_put(cma_kobj_root) should deal with it. No?
>>>
>> If this fails when i > 0, there will be cma_kobj instances that
>> were stashed in the cma_areas[] array. But this code only deletes
>> the most recently allocated cma_kobj, not anything allocated on
>> previous iterations of the loop.
>
> Oh, I misunderstood that destroying of root kobject will release
> children recursively. Seems not true. Go back to old version.
>
>
> index 16c81c9cb9b7..418951a3f138 100644
> --- a/mm/cma_sysfs.c
> +++ b/mm/cma_sysfs.c
> @@ -80,20 +80,19 @@ static struct kobj_type cma_ktype = {
> static int __init cma_sysfs_init(void)
> {
> unsigned int i;
> + int err;
> + struct cma *cma;
> + struct cma_kobject *cma_kobj;
>
> cma_kobj_root = kobject_create_and_add("cma", mm_kobj);
> if (!cma_kobj_root)
> return -ENOMEM;
>
> for (i = 0; i < cma_area_count; i++) {
> - int err;
> - struct cma *cma;
> - struct cma_kobject *cma_kobj;
> -
> cma_kobj = kzalloc(sizeof(*cma_kobj), GFP_KERNEL);
> if (!cma_kobj) {
> - kobject_put(cma_kobj_root);
> - return -ENOMEM;
> + err = -ENOMEM;
> + goto out;
> }
>
> cma = _areas[i];
> @@ -103,11 +102,21 @@ static int __init cma_sysfs_init(void)
>cma_kobj_root, "%s", cma->name);
> if (err) {
> kobject_put(_kobj->kobj);
> - kobject_put(cma_kobj_root);
> - return err;
> + goto out;
> }
> }
>
> return 0;
> +out:
> + while (--i >= 0) {
> + cma = _areas[i];
> +
> + kobject_put(>kobj->kobj);
> + kfree(cma->kobj);
> + cma->kobj = NULL;
> + }
> + kobject_put(cma_kobj_root);
> +
> + return err;
> }
> subsys_initcall(cma_sysfs_init);
Since we don't care about the order in which kobjects are put, I'd write it in
this way, which I think looks cleaner:
static void cma_sysfs_cleanup(struct kobject *cma_kobj_root)
{
struct cma *cma = cma_areas;
unsigned int i;
for (i = 0; i < cma_area_count; i++, cma++) {
if (!cma->kobj)
break;
kobject_put(>kobj->kobj);
}
kobject_put(cma_kobj_root);
}
static int __init cma_sysfs_init(void)
{
struct kobject *cma_kobj_root;
unsigned int i;
cma_kobj_root = kobject_create_and_add("cma", mm_kobj);
if (!cma_kobj_root)
return -ENOMEM;
for (i = 0; i < cma_area_count; i++) {
struct cma_kobject *cma_kobj;
struct cma *cma;
int err;
cma_kobj = kzalloc(sizeof(*cma_kobj), GFP_KERNEL);
if (!cma_kobj) {
cma_sysfs_cleanup(cma_kobj_root);
return -ENOMEM;
}
cma = _areas[i];
cma->kobj = cma_kobj;
cma_kobj->cma = cma;
err = kobject_init_and_add(_kobj->kobj, _ktype,
cma_kobj_root, "%s", cma->name);
if (err) {
cma_sysfs_cleanup(cma_kobj_root);
return err;
}
}
return 0;
}
subsys_initcall(cma_sysfs_init);
Re: [PATCH v6] mm: cma: support sysfs
24.03.2021 04:05, Minchan Kim пишет:
> +static struct kobject *cma_kobj_root;
This should be a local variable.
> +static struct kobj_type cma_ktype = {
> + .release = cma_kobj_release,
> + .sysfs_ops = _sysfs_ops,
> + .default_groups = cma_groups
I'd add a comma to the end, for consistency.
> +};
Re: [PATCH v4 3/6] dt-bindings: power: tegra: Add binding for core power domain
24.03.2021 01:48, Rob Herring пишет:
> On Sun, Mar 14, 2021 at 07:48:07PM +0300, Dmitry Osipenko wrote:
>> All NVIDIA Tegra SoCs have a core power domain where majority of hardware
>> blocks reside. Add binding for the core power domain.
>>
>> Signed-off-by: Dmitry Osipenko
>> ---
>> .../power/nvidia,tegra20-core-domain.yaml | 51 +++
>> 1 file changed, 51 insertions(+)
>> create mode 100644
>> Documentation/devicetree/bindings/power/nvidia,tegra20-core-domain.yaml
>>
>> diff --git
>> a/Documentation/devicetree/bindings/power/nvidia,tegra20-core-domain.yaml
>> b/Documentation/devicetree/bindings/power/nvidia,tegra20-core-domain.yaml
>> new file mode 100644
>> index ..4692489d780a
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/power/nvidia,tegra20-core-domain.yaml
>> @@ -0,0 +1,51 @@
>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/power/nvidia,tegra20-core-domain.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: NVIDIA Tegra Core Power Domain
>> +
>> +maintainers:
>> + - Dmitry Osipenko
>> + - Jon Hunter
>> + - Thierry Reding
>> +
>> +allOf:
>> + - $ref: power-domain.yaml#
>> +
>> +properties:
>> + compatible:
>> +enum:
>> + - nvidia,tegra20-core-domain
>> + - nvidia,tegra30-core-domain
>> +
>> + operating-points-v2:
>> +description:
>> + Should contain level, voltages and opp-supported-hw property.
>> + The supported-hw is a bitfield indicating SoC speedo or process
>> + ID mask.
>> +
>> + "#power-domain-cells":
>> +const: 0
>> +
>> + power-supply:
>> +description:
>> + Phandle to voltage regulator connected to the SoC Core power rail.
>> +
>> +required:
>> + - compatible
>> + - operating-points-v2
>> + - "#power-domain-cells"
>> + - power-supply
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> + - |
>> +power-domain {
>> +compatible = "nvidia,tegra20-core-domain";
>> +operating-points-v2 = <_table>;
>> +power-supply = <>;
>> +#power-domain-cells = <0>;
>
> AFAICT, there's no way to access this 'hardware'?
correct
Re: [PATCH v5] mm: cma: support sysfs
23.03.2021 22:50, Minchan Kim пишет: > +#ifdef CONFIG_CMA_SYSFS > +void cma_sysfs_alloc_pages_count(struct cma *cma, size_t count); > +void cma_sysfs_fail_pages_count(struct cma *cma, size_t count); I'd also rename cma_sysfs_alloc_pages_count to cma_sysfs_account_success_pages and cma_sysfs_fail_pages_count to cma_sysfs_account_fail_pages.
Re: [PATCH v5] mm: cma: support sysfs
24.03.2021 00:19, Dmitry Osipenko пишет:
>> +if (!kobj)
>> +goto out;
>> +
>> +kobj->cma = cma;
>> +cma->kobj = kobj;
>> +if (kobject_init_and_add(>kobj->kobj, _ktype,
>> + cma_kobj_root, "%s", cma->name)) {
>> +kobject_put(>kobj->kobj);
>> +goto out;
>> +}
>> +}
>> +
>> +return 0;
>> +out:
>> +kobject_put(cma_kobj_root);
>> +
>> +return -ENOMEM;
> kobject_init_and_add returns a error code, it could be different from
> ENOMEM. Won't hurt to propagate the proper error code.
>
I think it will be cleaner to write it like this:
cma_kobj = kzalloc(sizeof(*cma_kobj), GFP_KERNEL);
if (!cma_kobj) {
kobject_put(cma_kobj_root);
return -ENOMEM;
}
cma_kobj->cma = cma;
err = kobject_init_and_add(_kobj->kobj, _ktype,
cma_kobj_root, "%s", cma->name);
if (err) {
kobject_put(_kobj->kobj);
kobject_put(cma_kobj_root);
return err;
}
}
return 0;
}
Re: [PATCH v5] mm: cma: support sysfs
23.03.2021 22:50, Minchan Kim пишет:
> Since CMA is getting used more widely, it's more important to
> keep monitoring CMA statistics for system health since it's
> directly related to user experience.
>
> This patch introduces sysfs statistics for CMA, in order to provide
> some basic monitoring of the CMA allocator.
>
> * the number of CMA page successful allocations
> * the number of CMA page allocation failures
>
> These two values allow the user to calcuate the allocation
typo: calculate
> struct cma {
> unsigned long base_pfn;
> @@ -16,6 +22,14 @@ struct cma {
> struct debugfs_u32_array dfs_bitmap;
> #endif
> char name[CMA_MAX_NAME];
> +#ifdef CONFIG_CMA_SYSFS
> + /* the number of CMA page successful allocations */
> + atomic64_t nr_pages_succeeded;
> + /* the number of CMA page allocation failures */
> + atomic64_t nr_pages_failed;
> + /* kobject requires dynamic objecjt */
typo: object
...
> +static void cma_kobj_release(struct kobject *kobj)
> +{
> + struct cma_kobject *cma_kobj =
> + container_of(kobj, struct cma_kobject, kobj);
I'd add a to_cma_kobject() helper to improve readability.
> + struct cma *cma = cma_kobj->cma;
> +
> + kfree(cma_kobj);
> + cma->kobj = NULL;
> +}
> +
> +static struct attribute *cma_attrs[] = {
> + _pages_success_attr.attr,
> + _pages_fail_attr.attr,
> + NULL,
> +};
> +ATTRIBUTE_GROUPS(cma);
> +
> +static struct kobject *cma_kobj_root;
> +
> +static struct kobj_type cma_ktype = {
> + .release = cma_kobj_release,
> + .sysfs_ops = _sysfs_ops,
> + .default_groups = cma_groups
> +};
> +
> +static int __init cma_sysfs_init(void)
> +{
> + int i = 0;
unsigned int, for consistency
There is no need to initialize this variable.
> + struct cma *cma;
> +
> + cma_kobj_root = kobject_create_and_add("cma", mm_kobj);
> + if (!cma_kobj_root)
> + return -ENOMEM;
> +
> + for (i = 0; i < cma_area_count; i++) {
> + struct cma_kobject *kobj;
> +
> + cma = _areas[i];
> + kobj = kzalloc(sizeof(struct cma_kobject), GFP_KERNEL);
Checkpatch should warn that kzalloc(*kobj, ..) is a better variant.
I'd also rename kobj to cma_kobj everywhere, for clarity.
> + if (!kobj)
> + goto out;
> +
> + kobj->cma = cma;
> + cma->kobj = kobj;
> + if (kobject_init_and_add(>kobj->kobj, _ktype,
> + cma_kobj_root, "%s", cma->name)) {
> + kobject_put(>kobj->kobj);
> + goto out;
> + }
> + }
> +
> + return 0;
> +out:
> + kobject_put(cma_kobj_root);
> +
> + return -ENOMEM;
kobject_init_and_add returns a error code, it could be different from
ENOMEM. Won't hurt to propagate the proper error code.
[PATCH v1 2/2] memory: tegra20: Protect debug code with a lock
Simultaneous accesses to MC_STAT h/w shouldn't be allowed since one
collection process stomps on another. There is no good reason for
polling stats in parallel in practice, nevertheless let's add a
protection lock, just for consistency.
Signed-off-by: Dmitry Osipenko
---
drivers/memory/tegra/tegra20.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/memory/tegra/tegra20.c b/drivers/memory/tegra/tegra20.c
index 4659c0cea30d..2db68a913b7a 100644
--- a/drivers/memory/tegra/tegra20.c
+++ b/drivers/memory/tegra/tegra20.c
@@ -5,6 +5,7 @@
#include
#include
+#include
#include
#include
#include
@@ -55,6 +56,8 @@
/* we store collected statistics as a fixed point values */
#define MC_FX_FRAC_SCALE 100
+static DEFINE_MUTEX(tegra20_mc_stat_lock);
+
struct tegra20_mc_stat_gather {
unsigned int pri_filter;
unsigned int pri_event;
@@ -615,8 +618,12 @@ static int tegra20_mc_stats_show(struct seq_file *s, void
*unused)
if (!stats)
return -ENOMEM;
+ mutex_lock(_mc_stat_lock);
+
tegra20_mc_collect_stats(mc, stats);
+ mutex_unlock(_mc_stat_lock);
+
seq_puts(s, "Memory client Events Timeout High priority
Bandwidth ARB RW change Successive Page miss\n");
seq_puts(s,
"-\n");
--
2.30.2
[PATCH v1 1/2] memory: tegra20: Correct comment to MC_STAT registers writes
The code was changed multiple times and the comment to MC_STAT registers writes became slightly outdated. The MC_STAT programming now isn't hardcoded to the "bandwidth" mode, let's clarify this in the comment. Signed-off-by: Dmitry Osipenko --- drivers/memory/tegra/tegra20.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/memory/tegra/tegra20.c b/drivers/memory/tegra/tegra20.c index 14caf5b9324c..4659c0cea30d 100644 --- a/drivers/memory/tegra/tegra20.c +++ b/drivers/memory/tegra/tegra20.c @@ -451,9 +451,8 @@ static void tegra20_mc_stat_gather(struct tegra20_mc_stat *stat) control_1 = tegra20_mc_stat_gather_control(>gather1); /* -* Reset statistic gathers state, select bandwidth mode for the -* statistics collection mode and set clocks counter saturation -* limit to maximum. +* Reset statistic gathers state, select statistics collection mode +* and set clocks counter saturation limit to maximum. */ mc_writel(mc, 0x, MC_STAT_CONTROL); mc_writel(mc, control_0, MC_STAT_EMC_CONTROL_0); -- 2.30.2
Re: [PATCH v4] mm: cma: support sysfs
20.03.2021 10:52, Greg Kroah-Hartman пишет: .. >> I found the Greg's original argument and not sure that it's really >> worthwhile to worry about the copycats since this is not a driver's code.. >> >> Maybe we could just add a clarifying comment for the kobj, telling why >> it's okay for CMA. Greg, doesn't it sound like a good compromise to you? > > Please no. > In the case of a static objects, like CMA, this creates more bad than good, IMO. Even experienced developers are getting confused. In the end it's up to you guys to decide what to choose, either way will work.
Re: [PATCH v6 0/3] Support wakeup methods of Atmel maXTouch controllers
22.03.2021 01:44, Dmitry Torokhov пишет: > Hi Dmitry, > > On Sat, Mar 20, 2021 at 07:02:43PM +0300, Dmitry Osipenko wrote: >> 02.03.2021 13:21, Dmitry Osipenko пишет: >>> Some Atmel maXTouch controllers, like mXT1386 and mXT3432S1 for example, >>> have a WAKE line that needs to be asserted in order to wake controller >>> from a deep sleep, otherwise it will be unusable. This series implements >>> support for the wakeup methods in accordance to the mXT1386 datasheet [1], >>> see page 29 (chapter "5.8 WAKE Line"). >>> >>> The mXT1386 is a widely used controller found on many older Android tablet >>> devices. Touchscreen on Acer A500 tablet now works properly after this >>> series. >>> >>> This patchset is a continuation of the work originally started by >>> Jiada Wang [2]. >>> >>> [1] >>> https://ww1.microchip.com/downloads/en/DeviceDoc/mXT1386_1vx_Datasheet_LX.pdf >>> [2] https://patchwork.kernel.org/project/linux-input/list/?series=357875 >> >> Hi, >> >> This series is very wanted by Android tablet devices from Acer, Asus and >> other vendors which use Maxtouch 1386 controller. Touchscreens don't >> work without the wakeup support, i.e. without this series. The wakeup >> support is implemented in accordance to the datasheet and touchscreens >> are working excellent using these patches. >> >> Could you please take this series into v5.13? >> >> Or could you please let me know what exactly needs to be improved? > > Sorry, I was still slightly unhappy that we still are not tracking the > state of controller and opportunistically retrying failed I2C transfers, > but as I am failing to find time to come up with another solution I have > just applied your series. Thank you! I don't have other solutions either, although /I think/ potentially it should be possible to differentiate the I2C error here. On NVIDIA Tegra I see that I2C controller always gets a h/w NAK on TS wake-up and it returns -EREMOTEIO in this case. IIRC, some other non-NVIDIA I2C drivers always return -EIO on any error, so this method isn't universal, but certainly it feels like there is some a room for further improvements.
Re: [PATCH v6 0/3] Support wakeup methods of Atmel maXTouch controllers
02.03.2021 13:21, Dmitry Osipenko пишет: > Some Atmel maXTouch controllers, like mXT1386 and mXT3432S1 for example, > have a WAKE line that needs to be asserted in order to wake controller > from a deep sleep, otherwise it will be unusable. This series implements > support for the wakeup methods in accordance to the mXT1386 datasheet [1], > see page 29 (chapter "5.8 WAKE Line"). > > The mXT1386 is a widely used controller found on many older Android tablet > devices. Touchscreen on Acer A500 tablet now works properly after this > series. > > This patchset is a continuation of the work originally started by > Jiada Wang [2]. > > [1] > https://ww1.microchip.com/downloads/en/DeviceDoc/mXT1386_1vx_Datasheet_LX.pdf > [2] https://patchwork.kernel.org/project/linux-input/list/?series=357875 Hi, This series is very wanted by Android tablet devices from Acer, Asus and other vendors which use Maxtouch 1386 controller. Touchscreens don't work without the wakeup support, i.e. without this series. The wakeup support is implemented in accordance to the datasheet and touchscreens are working excellent using these patches. Could you please take this series into v5.13? Or could you please let me know what exactly needs to be improved? Thanks in advance.
[PATCH v6 2/7] clk: tegra: Fix refcounting of gate clocks
The refcounting of the gate clocks has a bug causing the enable_refcnt
to underflow when unused clocks are disabled. This happens because clk
provider erroneously bumps the refcount if clock is enabled at a boot
time, which it shouldn't be doing, and it does this only for the gate
clocks, while peripheral clocks are using the same gate ops and the
peripheral clocks are missing the initial bump. Hence the refcount of
the peripheral clocks is 0 when unused clocks are disabled and then the
counter is decremented further by the gate ops, causing the integer
underflow.
Fix this problem by removing the erroneous bump and by implementing the
disable_unused() callback, which disables the unused gates properly.
The visible effect of the bug is such that the unused clocks are never
gated if a loaded kernel module grabs the unused clocks and starts to use
them. In practice this shouldn't cause any real problems for the drivers
and boards supported by the kernel today.
Acked-by: Thierry Reding
Signed-off-by: Dmitry Osipenko
---
drivers/clk/tegra/clk-periph-gate.c | 72 +++--
drivers/clk/tegra/clk-periph.c | 11 +
2 files changed, 58 insertions(+), 25 deletions(-)
diff --git a/drivers/clk/tegra/clk-periph-gate.c
b/drivers/clk/tegra/clk-periph-gate.c
index 4b31beefc9fc..dc3f92678407 100644
--- a/drivers/clk/tegra/clk-periph-gate.c
+++ b/drivers/clk/tegra/clk-periph-gate.c
@@ -48,18 +48,9 @@ static int clk_periph_is_enabled(struct clk_hw *hw)
return state;
}
-static int clk_periph_enable(struct clk_hw *hw)
+static void clk_periph_enable_locked(struct clk_hw *hw)
{
struct tegra_clk_periph_gate *gate = to_clk_periph_gate(hw);
- unsigned long flags = 0;
-
- spin_lock_irqsave(_ref_lock, flags);
-
- gate->enable_refcnt[gate->clk_num]++;
- if (gate->enable_refcnt[gate->clk_num] > 1) {
- spin_unlock_irqrestore(_ref_lock, flags);
- return 0;
- }
write_enb_set(periph_clk_to_bit(gate), gate);
udelay(2);
@@ -78,6 +69,32 @@ static int clk_periph_enable(struct clk_hw *hw)
udelay(1);
writel_relaxed(0, gate->clk_base + LVL2_CLK_GATE_OVRE);
}
+}
+
+static void clk_periph_disable_locked(struct clk_hw *hw)
+{
+ struct tegra_clk_periph_gate *gate = to_clk_periph_gate(hw);
+
+ /*
+* If peripheral is in the APB bus then read the APB bus to
+* flush the write operation in apb bus. This will avoid the
+* peripheral access after disabling clock
+*/
+ if (gate->flags & TEGRA_PERIPH_ON_APB)
+ tegra_read_chipid();
+
+ write_enb_clr(periph_clk_to_bit(gate), gate);
+}
+
+static int clk_periph_enable(struct clk_hw *hw)
+{
+ struct tegra_clk_periph_gate *gate = to_clk_periph_gate(hw);
+ unsigned long flags = 0;
+
+ spin_lock_irqsave(_ref_lock, flags);
+
+ if (!gate->enable_refcnt[gate->clk_num]++)
+ clk_periph_enable_locked(hw);
spin_unlock_irqrestore(_ref_lock, flags);
@@ -91,21 +108,28 @@ static void clk_periph_disable(struct clk_hw *hw)
spin_lock_irqsave(_ref_lock, flags);
- gate->enable_refcnt[gate->clk_num]--;
- if (gate->enable_refcnt[gate->clk_num] > 0) {
- spin_unlock_irqrestore(_ref_lock, flags);
- return;
- }
+ WARN_ON(!gate->enable_refcnt[gate->clk_num]);
+
+ if (--gate->enable_refcnt[gate->clk_num] == 0)
+ clk_periph_disable_locked(hw);
+
+ spin_unlock_irqrestore(_ref_lock, flags);
+}
+
+static void clk_periph_disable_unused(struct clk_hw *hw)
+{
+ struct tegra_clk_periph_gate *gate = to_clk_periph_gate(hw);
+ unsigned long flags = 0;
+
+ spin_lock_irqsave(_ref_lock, flags);
/*
-* If peripheral is in the APB bus then read the APB bus to
-* flush the write operation in apb bus. This will avoid the
-* peripheral access after disabling clock
+* Some clocks are duplicated and some of them are marked as critical,
+* like fuse and fuse_burn for example, thus the enable_refcnt will
+* be non-zero here if the "unused" duplicate is disabled by CCF.
*/
- if (gate->flags & TEGRA_PERIPH_ON_APB)
- tegra_read_chipid();
-
- write_enb_clr(periph_clk_to_bit(gate), gate);
+ if (!gate->enable_refcnt[gate->clk_num])
+ clk_periph_disable_locked(hw);
spin_unlock_irqrestore(_ref_lock, flags);
}
@@ -114,6 +138,7 @@ const struct clk_ops tegra_clk_periph_gate_ops = {
.is_enabled = clk_periph_is_enabled,
.enable = clk_periph_enable,
.disable = clk_periph_disable,
+ .disable_unused = clk_periph_disable_unused,
};
struct clk *tegra_clk_register_periph_gate(const char *name,
@@ -148,9 +173,6 @@ struct clk *tegra_clk_register_periph
[PATCH v6 5/7] MAINTAINERS: Hand Tegra clk driver to Jon and Thierry
Peter and Prashant aren't actively maintaining Tegra clock driver anymore. Jonathan and Thierry will pick up maintaining of the driver from now on. Acked-by: Thierry Reding Signed-off-by: Dmitry Osipenko --- CREDITS | 6 ++ MAINTAINERS | 4 ++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/CREDITS b/CREDITS index cf8e23498a34..5577a2bdd93a 100644 --- a/CREDITS +++ b/CREDITS @@ -1250,6 +1250,10 @@ S: 29 Duchifat St. S: Ra'anana 4372029 S: Israel +N: Prashant Gaikwad +E: pgaik...@nvidia.com +D: Maintained NVIDIA Tegra clock driver + N: Kumar Gala E: ga...@kernel.crashing.org D: Embedded PowerPC 6xx/7xx/74xx/82xx/83xx/85xx support @@ -3387,7 +3391,9 @@ E: D: Macintosh IDE Driver N: Peter De Schrijver +E: pdeschrij...@nvidia.com E: stu...@cc4.kuleuven.ac.be +D: Maintained NVIDIA Tegra clock driver D: Mitsumi CD-ROM driver patches March version S: Molenbaan 29 S: B2240 Zandhoven diff --git a/MAINTAINERS b/MAINTAINERS index 08f9c2b7f3b3..830ade14ee68 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -17681,8 +17681,8 @@ T: git git://git.kernel.org/pub/scm/linux/kernel/git/tegra/linux.git N: [^a-z]tegra TEGRA CLOCK DRIVER -M: Peter De Schrijver -M: Prashant Gaikwad +M: Jonathan Hunter +M: Thierry Reding S: Supported F: drivers/clk/tegra/ -- 2.30.2
[PATCH v6 6/7] clk: tegra: Don't allow zero clock rate for PLLs
Zero clock rate doesn't make sense for PLLs and tegra-clk driver enters
into infinite loop on trying to calculate PLL parameters for zero rate.
Make code to error out if requested rate is zero.
Originally this trouble was found by Robert Yang while he was trying to
bring up upstream kernel on Samsung Galaxy Tab, which happened due to a
bug in Tegra DRM driver that erroneously sets PLL rate to zero. This
issues came over again recently during of kernel bring up on ASUS TF700T.
Signed-off-by: Dmitry Osipenko
---
drivers/clk/tegra/clk-pll.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/clk/tegra/clk-pll.c b/drivers/clk/tegra/clk-pll.c
index d709ecb7d8d7..af7d4941042e 100644
--- a/drivers/clk/tegra/clk-pll.c
+++ b/drivers/clk/tegra/clk-pll.c
@@ -558,6 +558,9 @@ static int _calc_rate(struct clk_hw *hw, struct
tegra_clk_pll_freq_table *cfg,
u32 p_div = 0;
int ret;
+ if (!rate)
+ return -EINVAL;
+
switch (parent_rate) {
case 1200:
case 2600:
--
2.30.2
[PATCH v6 7/7] dt-bindings: clock: tegra: Convert to schema
Convert NVIDIA Tegra clock bindings to schema.
Signed-off-by: Dmitry Osipenko
---
.../bindings/clock/nvidia,tegra114-car.txt| 63 --
.../bindings/clock/nvidia,tegra124-car.txt| 107
.../bindings/clock/nvidia,tegra124-car.yaml | 115 ++
.../bindings/clock/nvidia,tegra20-car.txt | 63 --
.../bindings/clock/nvidia,tegra20-car.yaml| 69 +++
.../bindings/clock/nvidia,tegra210-car.txt| 56 -
.../bindings/clock/nvidia,tegra30-car.txt | 63 --
7 files changed, 184 insertions(+), 352 deletions(-)
delete mode 100644
Documentation/devicetree/bindings/clock/nvidia,tegra114-car.txt
delete mode 100644
Documentation/devicetree/bindings/clock/nvidia,tegra124-car.txt
create mode 100644
Documentation/devicetree/bindings/clock/nvidia,tegra124-car.yaml
delete mode 100644
Documentation/devicetree/bindings/clock/nvidia,tegra20-car.txt
create mode 100644
Documentation/devicetree/bindings/clock/nvidia,tegra20-car.yaml
delete mode 100644
Documentation/devicetree/bindings/clock/nvidia,tegra210-car.txt
delete mode 100644
Documentation/devicetree/bindings/clock/nvidia,tegra30-car.txt
diff --git a/Documentation/devicetree/bindings/clock/nvidia,tegra114-car.txt
b/Documentation/devicetree/bindings/clock/nvidia,tegra114-car.txt
deleted file mode 100644
index 9acea9d93160..
--- a/Documentation/devicetree/bindings/clock/nvidia,tegra114-car.txt
+++ /dev/null
@@ -1,63 +0,0 @@
-NVIDIA Tegra114 Clock And Reset Controller
-
-This binding uses the common clock binding:
-Documentation/devicetree/bindings/clock/clock-bindings.txt
-
-The CAR (Clock And Reset) Controller on Tegra is the HW module responsible
-for muxing and gating Tegra's clocks, and setting their rates.
-
-Required properties :
-- compatible : Should be "nvidia,tegra114-car"
-- reg : Should contain CAR registers location and length
-- clocks : Should contain phandle and clock specifiers for two clocks:
- the 32 KHz "32k_in", and the board-specific oscillator "osc".
-- #clock-cells : Should be 1.
- In clock consumers, this cell represents the clock ID exposed by the
- CAR. The assignments may be found in header file
- .
-- #reset-cells : Should be 1.
- In clock consumers, this cell represents the bit number in the CAR's
- array of CLK_RST_CONTROLLER_RST_DEVICES_* registers.
-
-Example SoC include file:
-
-/ {
- tegra_car: clock {
- compatible = "nvidia,tegra114-car";
- reg = <0x60006000 0x1000>;
- #clock-cells = <1>;
- #reset-cells = <1>;
- };
-
- usb@c5004000 {
- clocks = <_car TEGRA114_CLK_USB2>;
- };
-};
-
-Example board file:
-
-/ {
- clocks {
- compatible = "simple-bus";
- #address-cells = <1>;
- #size-cells = <0>;
-
- osc: clock@0 {
- compatible = "fixed-clock";
- reg = <0>;
- #clock-cells = <0>;
- clock-frequency = <1200>;
- };
-
- clk_32k: clock@1 {
- compatible = "fixed-clock";
- reg = <1>;
- #clock-cells = <0>;
- clock-frequency = <32768>;
- };
- };
-
- _car {
- clocks = <_32k> <>;
- };
-};
diff --git a/Documentation/devicetree/bindings/clock/nvidia,tegra124-car.txt
b/Documentation/devicetree/bindings/clock/nvidia,tegra124-car.txt
deleted file mode 100644
index 7f02fb4ca4ad..
--- a/Documentation/devicetree/bindings/clock/nvidia,tegra124-car.txt
+++ /dev/null
@@ -1,107 +0,0 @@
-NVIDIA Tegra124 and Tegra132 Clock And Reset Controller
-
-This binding uses the common clock binding:
-Documentation/devicetree/bindings/clock/clock-bindings.txt
-
-The CAR (Clock And Reset) Controller on Tegra is the HW module responsible
-for muxing and gating Tegra's clocks, and setting their rates.
-
-Required properties :
-- compatible : Should be "nvidia,tegra124-car" or "nvidia,tegra132-car"
-- reg : Should contain CAR registers location and length
-- clocks : Should contain phandle and clock specifiers for two clocks:
- the 32 KHz "32k_in", and the board-specific oscillator "osc".
-- #clock-cells : Should be 1.
- In clock consumers, this cell represents the clock ID exposed by the
- CAR. The assignments may be found in the header files
- (which covers IDs common
- to Tegra124 and Tegra132) and
- (for Tegra124-specific clocks).
-- #reset-cells : Should be 1.
- In clock consumers, this cell represents the bit number in the CAR's
- array of CLK_RST_CONTROLLER_RST_DEVICES_* registers.
-- nvidia,external-memory-controller : phandle of the
[PATCH v6 3/7] clk: tegra: Ensure that PLLU configuration is applied properly
The PLLU (USB) consists of the PLL configuration itself and configuration of the PLLU outputs. The PLLU programming is inconsistent on T30 vs T114, where T114 immediately bails out if PLLU is enabled and T30 re-enables a potentially already enabled PLL (left after bootloader) and then fully reprograms it, which could be unsafe to do. The correct way should be to skip enabling of the PLL if it's already enabled and then apply configuration to the outputs. This patch doesn't fix any known problems, it's a minor improvement. Acked-by: Thierry Reding Signed-off-by: Dmitry Osipenko --- drivers/clk/tegra/clk-pll.c | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/clk/tegra/clk-pll.c b/drivers/clk/tegra/clk-pll.c index c5cc0a2dac6f..d709ecb7d8d7 100644 --- a/drivers/clk/tegra/clk-pll.c +++ b/drivers/clk/tegra/clk-pll.c @@ -1131,7 +1131,8 @@ static int clk_pllu_enable(struct clk_hw *hw) if (pll->lock) spin_lock_irqsave(pll->lock, flags); - _clk_pll_enable(hw); + if (!clk_pll_is_enabled(hw)) + _clk_pll_enable(hw); ret = clk_pll_wait_for_lock(pll); if (ret < 0) @@ -1748,15 +1749,13 @@ static int clk_pllu_tegra114_enable(struct clk_hw *hw) return -EINVAL; } - if (clk_pll_is_enabled(hw)) - return 0; - input_rate = clk_hw_get_rate(__clk_get_hw(osc)); if (pll->lock) spin_lock_irqsave(pll->lock, flags); - _clk_pll_enable(hw); + if (!clk_pll_is_enabled(hw)) + _clk_pll_enable(hw); ret = clk_pll_wait_for_lock(pll); if (ret < 0) -- 2.30.2
[PATCH v6 4/7] clk: tegra: Halve SCLK rate on Tegra20
Higher SCLK rates on Tegra20 require high core voltage. The higher
clock rate may have a positive performance effect only for AHB DMA
transfers and AVP CPU, but both aren't used by upstream kernel at all.
Halve SCLK rate on Tegra20 in order to remove the high core voltage
requirement.
Acked-by: Thierry Reding
Signed-off-by: Dmitry Osipenko
---
drivers/clk/tegra/clk-tegra20.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/clk/tegra/clk-tegra20.c b/drivers/clk/tegra/clk-tegra20.c
index 3efc651b42e3..3664593a5ba4 100644
--- a/drivers/clk/tegra/clk-tegra20.c
+++ b/drivers/clk/tegra/clk-tegra20.c
@@ -1021,9 +1021,9 @@ static struct tegra_clk_init_table init_table[]
__initdata = {
{ TEGRA20_CLK_PLL_P_OUT3, TEGRA20_CLK_CLK_MAX, 7200, 1 },
{ TEGRA20_CLK_PLL_P_OUT4, TEGRA20_CLK_CLK_MAX, 2400, 1 },
{ TEGRA20_CLK_PLL_C, TEGRA20_CLK_CLK_MAX, 6, 0 },
- { TEGRA20_CLK_PLL_C_OUT1, TEGRA20_CLK_CLK_MAX, 24000, 0 },
- { TEGRA20_CLK_SCLK, TEGRA20_CLK_PLL_C_OUT1, 24000, 0 },
- { TEGRA20_CLK_HCLK, TEGRA20_CLK_CLK_MAX, 24000, 0 },
+ { TEGRA20_CLK_PLL_C_OUT1, TEGRA20_CLK_CLK_MAX, 12000, 0 },
+ { TEGRA20_CLK_SCLK, TEGRA20_CLK_PLL_C_OUT1, 12000, 0 },
+ { TEGRA20_CLK_HCLK, TEGRA20_CLK_CLK_MAX, 12000, 0 },
{ TEGRA20_CLK_PCLK, TEGRA20_CLK_CLK_MAX, 6000, 0 },
{ TEGRA20_CLK_CSITE, TEGRA20_CLK_CLK_MAX, 0, 1 },
{ TEGRA20_CLK_CCLK, TEGRA20_CLK_CLK_MAX, 0, 1 },
--
2.30.2
[PATCH v6 0/7] Couple improvements for Tegra clk driver
This series fixes couple minor standalone problems of the Tegra clk driver. Changelog: v6: - Made a small improvement and corrected a typo in patch "Fix refcounting of gate clocks" that were spotted by Michał Mirosław. v5: - Corrected example in the schema binding to silence dt_binding_check warning. - The Tegra124 binding is factored out into standalone binding since Tegra124 has properties that aren't used by other SoCs and I couldn't figure out how to make them conditional in schema. v4: - Added new patch that converts DT bindings to schema. v3: - Added acks from Thierry Reding that he gave to v2. - Added new patch "clk: tegra: Don't allow zero clock rate for PLLs". v2: - Added these new patches: clk: tegra: Halve SCLK rate on Tegra20 MAINTAINERS: Hand Tegra clk driver to Jon and Thierry v1: - Collected clk patches into a single series. Dmitry Osipenko (7): clk: tegra30: Use 300MHz for video decoder by default clk: tegra: Fix refcounting of gate clocks clk: tegra: Ensure that PLLU configuration is applied properly clk: tegra: Halve SCLK rate on Tegra20 MAINTAINERS: Hand Tegra clk driver to Jon and Thierry clk: tegra: Don't allow zero clock rate for PLLs dt-bindings: clock: tegra: Convert to schema CREDITS | 6 + .../bindings/clock/nvidia,tegra114-car.txt| 63 -- .../bindings/clock/nvidia,tegra124-car.txt| 107 .../bindings/clock/nvidia,tegra124-car.yaml | 115 ++ .../bindings/clock/nvidia,tegra20-car.txt | 63 -- .../bindings/clock/nvidia,tegra20-car.yaml| 69 +++ .../bindings/clock/nvidia,tegra210-car.txt| 56 - .../bindings/clock/nvidia,tegra30-car.txt | 63 -- MAINTAINERS | 4 +- drivers/clk/tegra/clk-periph-gate.c | 72 +++ drivers/clk/tegra/clk-periph.c| 11 ++ drivers/clk/tegra/clk-pll.c | 12 +- drivers/clk/tegra/clk-tegra20.c | 6 +- drivers/clk/tegra/clk-tegra30.c | 2 +- 14 files changed, 261 insertions(+), 388 deletions(-) delete mode 100644 Documentation/devicetree/bindings/clock/nvidia,tegra114-car.txt delete mode 100644 Documentation/devicetree/bindings/clock/nvidia,tegra124-car.txt create mode 100644 Documentation/devicetree/bindings/clock/nvidia,tegra124-car.yaml delete mode 100644 Documentation/devicetree/bindings/clock/nvidia,tegra20-car.txt create mode 100644 Documentation/devicetree/bindings/clock/nvidia,tegra20-car.yaml delete mode 100644 Documentation/devicetree/bindings/clock/nvidia,tegra210-car.txt delete mode 100644 Documentation/devicetree/bindings/clock/nvidia,tegra30-car.txt -- 2.30.2
[PATCH v6 1/7] clk: tegra30: Use 300MHz for video decoder by default
The 600MHz is a too high clock rate for some SoC versions for the video
decoder hardware and this may cause stability issues. Use 300MHz for the
video decoder by default, which is supported by all hardware versions.
Fixes: ed1a2459e20c ("clk: tegra: Add Tegra20/30 EMC clock implementation")
Acked-by: Thierry Reding
Signed-off-by: Dmitry Osipenko
---
drivers/clk/tegra/clk-tegra30.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/clk/tegra/clk-tegra30.c b/drivers/clk/tegra/clk-tegra30.c
index 16dbf83d2f62..a33688b2359e 100644
--- a/drivers/clk/tegra/clk-tegra30.c
+++ b/drivers/clk/tegra/clk-tegra30.c
@@ -1245,7 +1245,7 @@ static struct tegra_clk_init_table init_table[]
__initdata = {
{ TEGRA30_CLK_GR3D, TEGRA30_CLK_PLL_C, 3, 0 },
{ TEGRA30_CLK_GR3D2, TEGRA30_CLK_PLL_C, 3, 0 },
{ TEGRA30_CLK_PLL_U, TEGRA30_CLK_CLK_MAX, 48000, 0 },
- { TEGRA30_CLK_VDE, TEGRA30_CLK_PLL_C, 6, 0 },
+ { TEGRA30_CLK_VDE, TEGRA30_CLK_PLL_C, 3, 0 },
{ TEGRA30_CLK_SPDIF_IN_SYNC, TEGRA30_CLK_CLK_MAX, 2400, 0 },
{ TEGRA30_CLK_I2S0_SYNC, TEGRA30_CLK_CLK_MAX, 2400, 0 },
{ TEGRA30_CLK_I2S1_SYNC, TEGRA30_CLK_CLK_MAX, 2400, 0 },
--
2.30.2
[PATCH v1 2/2] usb: host: ehci-tegra: Select USB_GADGET Kconfig option
Select USB_GADGET Kconfig option in order to fix build failure which
happens because ChipIdea driver has a build dependency on both USB_GADGET
and USB_EHCI_HCD, while USB_EHCI_TEGRA force-selects the ChipIdea driver
without taking into account the tristate USB_GADGET dependency. It's not
possible to do anything about the cyclic dependency of the Kconfig
options, but USB_EHCI_TEGRA is now a deprecated option that isn't used
by defconfigs and USB_GADGET is wanted on Tegra by default, hence it's
okay to have a bit clunky workaround for it.
Fixes: c3590c7656fb ("usb: host: ehci-tegra: Remove the driver")
Reported-by: kernel test robot
Signed-off-by: Dmitry Osipenko
---
drivers/usb/host/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index b94f2a070c05..df9428f1dc5e 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -272,6 +272,7 @@ config USB_EHCI_TEGRA
select USB_CHIPIDEA
select USB_CHIPIDEA_HOST
select USB_CHIPIDEA_TEGRA
+ select USB_GADGET
help
This option is deprecated now and the driver was removed, use
USB_CHIPIDEA_TEGRA instead.
--
2.30.2
[PATCH v1 1/2] ARM: multi_v7_defconfig: Stop using deprecated USB_EHCI_TEGRA
The USB_EHCI_TEGRA option is deprecated now and replaced by USB_CHIPIDEA_TEGRA. Replace USB_EHCI_TEGRA with USB_CHIPIDEA_TEGRA in multi_v7_defconfig. Signed-off-by: Dmitry Osipenko --- arch/arm/configs/multi_v7_defconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig index 3823da605430..d3242264514e 100644 --- a/arch/arm/configs/multi_v7_defconfig +++ b/arch/arm/configs/multi_v7_defconfig @@ -791,7 +791,6 @@ CONFIG_USB_XHCI_MVEBU=y CONFIG_USB_XHCI_TEGRA=m CONFIG_USB_EHCI_HCD=y CONFIG_USB_EHCI_HCD_STI=y -CONFIG_USB_EHCI_TEGRA=y CONFIG_USB_EHCI_EXYNOS=m CONFIG_USB_EHCI_MV=m CONFIG_USB_OHCI_HCD=y @@ -817,6 +816,7 @@ CONFIG_USB_DWC2=y CONFIG_USB_CHIPIDEA=y CONFIG_USB_CHIPIDEA_UDC=y CONFIG_USB_CHIPIDEA_HOST=y +CONFIG_USB_CHIPIDEA_TEGRA=y CONFIG_USB_ISP1760=y CONFIG_USB_HSIC_USB3503=y CONFIG_AB8500_USB=y -- 2.30.2
Re: [PATCH v4] mm: cma: support sysfs
19.03.2021 22:03, Minchan Kim пишет:
> On Fri, Mar 19, 2021 at 09:48:11PM +0300, Dmitry Osipenko wrote:
>> 19.03.2021 21:21, Minchan Kim пишет:
>>> On Fri, Mar 19, 2021 at 08:56:06PM +0300, Dmitry Osipenko wrote:
>>>> 19.03.2021 19:30, Minchan Kim пишет:
>>>>> +static void cma_kobj_release(struct kobject *kobj)
>>>>> +{
>>>>> + struct cma_kobject *cma_kobj = container_of(kobj, struct cma_kobject,
>>>>> kobj);
>>>>> +
>>>>> + kfree(cma_kobj);
>>>>> +}
>>>>
>>>> Oh, wait.. I think this kfree wrong since cma_kobj belongs to the array.
>>>
>>> Oh, good spot. Let me use kzalloc.
>>>
>>
>> Thinking a bit more about this.. it looks like actually it should be
>> better to get back to the older variant of cma_stat, but allocate at the
>> time of CMA initialization, rather than at the time of sysfs
>> initialization. Then the cma_stat will be decoupled from the cma struct
>
> IIRC, the problem was slab was not initiaized at CMA init point.
> That's why I liked your suggestion.
Alright, if CMA init time is a problem, then the recent variant should
be okay.
>> and cma_stat will be a self-contained object.
>
> Yeah, self-contained is better but it's already weird to
> have differnt lifetime for one object since CMA object
> never die, technically.
>
Indeed.
I found the Greg's original argument and not sure that it's really
worthwhile to worry about the copycats since this is not a driver's code..
Maybe we could just add a clarifying comment for the kobj, telling why
it's okay for CMA. Greg, doesn't it sound like a good compromise to you?
Re: [PATCH v4] mm: cma: support sysfs
19.03.2021 21:18, Minchan Kim пишет: >>> +#define CMA_ATTR_RO(_name) \ >>> + static struct kobj_attribute _name##_attr = __ATTR_RO(_name) >>> + >>> +static ssize_t alloc_pages_success_show(struct kobject *kobj, >>> + struct kobj_attribute *attr, char *buf) >> The indentations are still wrong. >> >> CHECK: Alignment should match open parenthesis > Hmm, I didn't know that we have that kinds of rule. > Maybe, my broken checkpatch couldn't catch it up. > Thanks. > > $ ./scripts/checkpatch.pl 0001-mm-cma-support-sysfs.patch > Traceback (most recent call last): > File "scripts/spdxcheck.py", line 6, in > from ply import lex, yacc > > > < snip > > That's probably because I use the --strict option of checkpatch by default.
Re: [PATCH v4] mm: cma: support sysfs
19.03.2021 21:18, Minchan Kim пишет:
>>> + if (ZERO_OR_NULL_PTR(cma_kobjs))
>>> + goto out;
>>> +
>>> + do {
>>> + cma = _areas[i];
>>> + cma->kobj = _kobjs[i];
>> Does cma really need are pointer to cma_kobj?
> Do you mean something like this?
>
> struct cma {
> ..
> ..
> struct kobject *kobj;
> };
>
> Then, how could we we make kobject dynamic model?
>
> We need to get struct cma from kboj like below.
>
> static ssize_t alloc_pages_success_show(struct kobject *kobj,
> struct kobj_attribute *attr, char
> *buf)
> {
> struct cma_kobject *cma_kobj = container_of(kobj,
> struct cma_kobject, kobj);
> struct cma *cma = cma_kobj->cma;
>
> return sysfs_emit(buf, "%llu\n",
> atomic64_read(>nr_pages_succeeded));
> }
>
> So kobj should be not a pointer in the container struct.
> Am I missing your point? Otherwise, it would be great if you suggest
> better idea.
I meant that cma_kobj->cma is okay, but cma->kobj wasn't really used
anywhere since struct cma can't be released. I.e. we could write
kobject_put(>kobj->kobj) as kobject_put(_kobjs[i].kobj);
But since we won't use the array anymore and maybe will get back to use
the cma_stat, then it will be fine to add the pointer.
Re: [PATCH v4] mm: cma: support sysfs
19.03.2021 21:21, Minchan Kim пишет:
> On Fri, Mar 19, 2021 at 08:56:06PM +0300, Dmitry Osipenko wrote:
>> 19.03.2021 19:30, Minchan Kim пишет:
>>> +static void cma_kobj_release(struct kobject *kobj)
>>> +{
>>> + struct cma_kobject *cma_kobj = container_of(kobj, struct cma_kobject,
>>> kobj);
>>> +
>>> + kfree(cma_kobj);
>>> +}
>>
>> Oh, wait.. I think this kfree wrong since cma_kobj belongs to the array.
>
> Oh, good spot. Let me use kzalloc.
>
Thinking a bit more about this.. it looks like actually it should be
better to get back to the older variant of cma_stat, but allocate at the
time of CMA initialization, rather than at the time of sysfs
initialization. Then the cma_stat will be decoupled from the cma struct
and cma_stat will be a self-contained object.
Re: [PATCH v4] mm: cma: support sysfs
19.03.2021 19:30, Minchan Kim пишет:
> +static void cma_kobj_release(struct kobject *kobj)
> +{
> + struct cma_kobject *cma_kobj = container_of(kobj, struct cma_kobject,
> kobj);
> +
> + kfree(cma_kobj);
> +}
Oh, wait.. I think this kfree wrong since cma_kobj belongs to the array.
Re: [PATCH v4] mm: cma: support sysfs
19.03.2021 20:41, Dmitry Osipenko пишет:
> 19.03.2021 20:29, Dmitry Osipenko пишет:
>> +void cma_sysfs_alloc_pages_count(struct cma *cma, size_t count)
>> +{
>> +atomic64_add(count, >nr_pages_succeeded);
>> +}
>> +
>> +void cma_sysfs_fail_pages_count(struct cma *cma, size_t count)
>> +{
>> +atomic64_add(count, >nr_pages_failed);
>> +}
>
> The atomic looks good, but aren't CMA allocations already protected by
> the CMA core? Do we really need to worry about racing here?
>
Although, please scratch that. I see now that these functions are called
outside of the lock.
