* Re: [PATCH v5 3/5] i2c: i2c-qcom-geni: Add bus driver for the Qualcomm GENI I2C controller
From: Doug Anderson @ 2018-03-23 23:34 UTC (permalink / raw)
To: Karthikeyan Ramasubramanian
Cc: Jonathan Corbet, Andy Gross, David Brown, Rob Herring,
Mark Rutland, Wolfram Sang, linux-doc, linux-arm-msm, devicetree,
linux-i2c, evgreen, acourbot, swboyd, Sagar Dharia,
Girish Mahadevan
In-Reply-To: <1521836461-6515-4-git-send-email-kramasub@codeaurora.org>
Hi,
On Fri, Mar 23, 2018 at 1:20 PM, Karthikeyan Ramasubramanian
<kramasub@codeaurora.org> wrote:
> This bus driver supports the GENI based i2c hardware controller in the
> Qualcomm SOCs. The Qualcomm Generic Interface (GENI) is a programmable
> module supporting a wide range of serial interfaces including I2C. The
> driver supports FIFO mode and DMA mode of transfer and switches modes
> dynamically depending on the size of the transfer.
>
> Signed-off-by: Karthikeyan Ramasubramanian <kramasub@codeaurora.org>
> Signed-off-by: Sagar Dharia <sdharia@codeaurora.org>
> Signed-off-by: Girish Mahadevan <girishm@codeaurora.org>
> ---
> drivers/i2c/busses/Kconfig | 13 +
> drivers/i2c/busses/Makefile | 1 +
> drivers/i2c/busses/i2c-qcom-geni.c | 650 +++++++++++++++++++++++++++++++++++++
> 3 files changed, 664 insertions(+)
[...]
> +/*
> + * Hardware uses the underlying formula to calculate time periods of
> + * SCL clock cycle. Firmware uses some additional cycles excluded from the
> + * below formula and it is confirmed that the time periods are within
> + * specification limits.
I was hoping for more than just "oh, and there's a fudge factor", but
I guess this is the best I'm going to get?
> +static int geni_i2c_probe(struct platform_device *pdev)
> +{
> + struct geni_i2c_dev *gi2c;
> + struct resource *res;
> + u32 proto, tx_depth;
> + int ret;
> +
> + gi2c = devm_kzalloc(&pdev->dev, sizeof(*gi2c), GFP_KERNEL);
> + if (!gi2c)
> + return -ENOMEM;
> +
> + gi2c->se.dev = &pdev->dev;
> + gi2c->se.wrapper = dev_get_drvdata(pdev->dev.parent);
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + gi2c->se.base = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(gi2c->se.base))
> + return PTR_ERR(gi2c->se.base);
> +
> + gi2c->se.clk = devm_clk_get(&pdev->dev, "se");
> + if (IS_ERR(gi2c->se.clk)) {
> + ret = PTR_ERR(gi2c->se.clk);
> + dev_err(&pdev->dev, "Err getting SE Core clk %d\n", ret);
> + return ret;
> + }
> +
> + ret = device_property_read_u32(&pdev->dev, "clock-frequency",
> + &gi2c->clk_freq_out);
> + if (ret) {
> + /* Clock frequency not specified, so default to 100kHz. */
> + dev_info(&pdev->dev,
> + "Bus frequency not specified, default to 100kHz.\n");
If you happen to spin again, can you remove the comment since it's
obvious from the string in the print? It looks a lot like this code:
/* Print hello, world */
printf("hello, world\n");
In any case, that's a pretty minor nit, so I'll add:
Reviewed-by: Douglas Anderson <dianders@chromium.org>
...assuming that the bindings and "geni" code get Acked / landed
somewhere. Ideally let's not land this before the geni code lands
since if the geni API changes for some reason it'll cause us grief.
-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v5 5/5] arm64: dts: sdm845: Add support for an instance of I2C controller
From: Doug Anderson @ 2018-03-23 23:38 UTC (permalink / raw)
To: Karthikeyan Ramasubramanian
Cc: Jonathan Corbet, Andy Gross, David Brown, Rob Herring,
Mark Rutland, Wolfram Sang, linux-doc, linux-arm-msm, devicetree,
linux-i2c, evgreen, acourbot, swboyd
In-Reply-To: <1521836461-6515-6-git-send-email-kramasub@codeaurora.org>
Hi,
On Fri, Mar 23, 2018 at 1:21 PM, Karthikeyan Ramasubramanian
<kramasub@codeaurora.org> wrote:
> + i2c10: i2c@a88000 {
> + compatible = "qcom,geni-i2c";
> + reg = <0xa88000 0x4000>;
> + clock-names = "se";
> + clocks = <&gcc GCC_QUPV3_WRAP1_S2_CLK>;
> + pinctrl-names = "default", "sleep";
> + pinctrl-0 = <&qup_i2c10_default>;
> + pinctrl-1 = <&qup_i2c10_sleep>;
> + interrupts = <GIC_SPI 355 IRQ_TYPE_LEVEL_HIGH>;
> + clock-frequency = <400000>;
Please move the clock-frequency to the board file. Not all devices on
all boards will support 400 kHz, so we should be at 100 kHz by default
and boards should explicitly say that they support 400 kHz.
Other than that things look good to me and you can add my Reviewed-by tag.
-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v4 7/8] clocksource: Add a new timer-ingenic driver
From: Daniel Lezcano @ 2018-03-24 6:26 UTC (permalink / raw)
To: Paul Cercueil, Thomas Gleixner, Jason Cooper, Marc Zyngier,
Lee Jones, Ralf Baechle, Rob Herring, Jonathan Corbet,
Mark Rutland
Cc: James Hogan, Maarten ter Huurne, linux-clk, devicetree,
linux-kernel, linux-mips, linux-doc
In-Reply-To: <20180317232901.14129-8-paul@crapouillou.net>
On 18/03/2018 00:29, Paul Cercueil wrote:
> This driver will use the TCU (Timer Counter Unit) present on the Ingenic
> JZ47xx SoCs to provide the kernel with a clocksource and timers.
Please provide a more detailed description about the timer.
Where is the clocksource ?
I don't see the point of using channel idx and pwm checking here.
There is one clockevent, why create multiple channels ? Can't you stick
to the usual init routine for a timer.
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---
> drivers/clocksource/Kconfig | 8 ++
> drivers/clocksource/Makefile | 1 +
> drivers/clocksource/timer-ingenic.c | 278 ++++++++++++++++++++++++++++++++++++
> 3 files changed, 287 insertions(+)
> create mode 100644 drivers/clocksource/timer-ingenic.c
>
> v2: Use SPDX identifier for the license
> v3: - Move documentation to its own patch
> - Search the devicetree for PWM clients, and use all the TCU
> channels that won't be used for PWM
> v4: - Add documentation about why we search for PWM clients
> - Verify that the PWM clients are for the TCU PWM driver
>
> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> index d2e5382821a4..481422145fb4 100644
> --- a/drivers/clocksource/Kconfig
> +++ b/drivers/clocksource/Kconfig
> @@ -592,4 +592,12 @@ config CLKSRC_ST_LPC
> Enable this option to use the Low Power controller timer
> as clocksource.
>
> +config INGENIC_TIMER
> + bool "Clocksource/timer using the TCU in Ingenic JZ SoCs"
> + depends on MACH_INGENIC || COMPILE_TEST
bool "Clocksource/timer using the TCU in Ingenic JZ SoCs" if COMPILE_TEST
Remove the depends MACH_INGENIC.
> + select CLKSRC_OF
> + default y
No default, Kconfig platform selects the timer.
> + help
> + Support for the timer/counter unit of the Ingenic JZ SoCs.
> +
> endmenu
> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
> index d6dec4489d66..98691e8999fe 100644
> --- a/drivers/clocksource/Makefile
> +++ b/drivers/clocksource/Makefile
> @@ -74,5 +74,6 @@ obj-$(CONFIG_ASM9260_TIMER) += asm9260_timer.o
> obj-$(CONFIG_H8300_TMR8) += h8300_timer8.o
> obj-$(CONFIG_H8300_TMR16) += h8300_timer16.o
> obj-$(CONFIG_H8300_TPU) += h8300_tpu.o
> +obj-$(CONFIG_INGENIC_TIMER) += timer-ingenic.o
> obj-$(CONFIG_CLKSRC_ST_LPC) += clksrc_st_lpc.o
> obj-$(CONFIG_X86_NUMACHIP) += numachip.o
> diff --git a/drivers/clocksource/timer-ingenic.c b/drivers/clocksource/timer-ingenic.c
> new file mode 100644
> index 000000000000..8c777c0c0023
> --- /dev/null
> +++ b/drivers/clocksource/timer-ingenic.c
> @@ -0,0 +1,278 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Ingenic JZ47xx SoC TCU clocksource driver
> + * Copyright (C) 2018 Paul Cercueil <paul@crapouillou.net>
> + */
> +
> +#include <linux/bitops.h>
> +#include <linux/clk.h>
> +#include <linux/clockchips.h>
> +#include <linux/err.h>
> +#include <linux/interrupt.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/mfd/syscon/ingenic-tcu.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/regmap.h>
> +#include <linux/slab.h>
> +
> +#define NUM_CHANNELS 8
> +
> +struct ingenic_tcu;
> +
> +struct ingenic_tcu_channel {
> + unsigned int idx;
> + struct clk *clk;
> +};
> +
> +struct ingenic_tcu {
> + struct ingenic_tcu_channel channels[NUM_CHANNELS];
> + unsigned long requested;
> + struct regmap *map;
> +};
> +
> +struct ingenic_clock_event_device {
> + struct clock_event_device cevt;
> + struct ingenic_tcu_channel *channel;
> + char name[32];
> +};
> +
> +#define ingenic_cevt(_evt) \
> + container_of(_evt, struct ingenic_clock_event_device, cevt)
> +
> +static inline struct ingenic_tcu *to_ingenic_tcu(struct ingenic_tcu_channel *ch)
> +{
> + return container_of(ch, struct ingenic_tcu, channels[ch->idx]);
> +}
> +
> +static int ingenic_tcu_cevt_set_state_shutdown(struct clock_event_device *evt)
> +{
> + struct ingenic_clock_event_device *jzcevt = ingenic_cevt(evt);
> + struct ingenic_tcu_channel *channel = jzcevt->channel;
> + struct ingenic_tcu *tcu = to_ingenic_tcu(channel);
> + unsigned int idx = channel->idx;
> +
> + regmap_write(tcu->map, TCU_REG_TECR, BIT(idx));
> + return 0;
> +}
> +
> +static int ingenic_tcu_cevt_set_next(unsigned long next,
> + struct clock_event_device *evt)
> +{
> + struct ingenic_clock_event_device *jzcevt = ingenic_cevt(evt);
> + struct ingenic_tcu_channel *channel = jzcevt->channel;
> + struct ingenic_tcu *tcu = to_ingenic_tcu(channel);
> + unsigned int idx = channel->idx;
> +
> + if (next > 0xffff)
> + return -EINVAL;
> +
> + regmap_write(tcu->map, TCU_REG_TDFRc(idx), (unsigned int) next);
> + regmap_write(tcu->map, TCU_REG_TCNTc(idx), 0);
> + regmap_write(tcu->map, TCU_REG_TESR, BIT(idx));
> +
> + return 0;
> +}
> +
> +static irqreturn_t ingenic_tcu_cevt_cb(int irq, void *dev_id)
> +{
> + struct clock_event_device *cevt = dev_id;
> + struct ingenic_clock_event_device *jzcevt = ingenic_cevt(cevt);
> + struct ingenic_tcu_channel *channel = jzcevt->channel;
> + struct ingenic_tcu *tcu = to_ingenic_tcu(channel);
> + unsigned int idx = channel->idx;
> +
> + regmap_write(tcu->map, TCU_REG_TECR, BIT(idx));
> +
> + if (cevt->event_handler)
> + cevt->event_handler(cevt);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int __init ingenic_tcu_req_channel(struct ingenic_tcu_channel *channel)
> +{
> + struct ingenic_tcu *tcu = to_ingenic_tcu(channel);
> + char buf[16];
> + int err;
> +
> + if (test_and_set_bit(channel->idx, &tcu->requested))
> + return -EBUSY;
> +
> + snprintf(buf, sizeof(buf), "timer%u", channel->idx);
> + channel->clk = clk_get(NULL, buf);
> + if (IS_ERR(channel->clk)) {
> + err = PTR_ERR(channel->clk);
> + goto out_release;
> + }
> +
> + err = clk_prepare_enable(channel->clk);
> + if (err)
> + goto out_clk_put;
> +
> + return 0;
> +
> +out_clk_put:
> + clk_put(channel->clk);
> +out_release:
> + clear_bit(channel->idx, &tcu->requested);
> + return err;
> +}
> +
> +static int __init ingenic_tcu_reset_channel(struct device_node *np,
> + struct ingenic_tcu_channel *channel)
> +{
> + struct ingenic_tcu *tcu = to_ingenic_tcu(channel);
> +
> + return regmap_update_bits(tcu->map, TCU_REG_TCSRc(channel->idx),
> + 0xffff & ~TCU_TCSR_RESERVED_BITS, 0);
> +}
> +
> +static void __init ingenic_tcu_free_channel(struct ingenic_tcu_channel *channel)
> +{
> + struct ingenic_tcu *tcu = to_ingenic_tcu(channel);
> +
> + clk_disable_unprepare(channel->clk);
> + clk_put(channel->clk);
> + clear_bit(channel->idx, &tcu->requested);
> +}
> +
> +static const char * const ingenic_tcu_timer_names[] = {
> + "TCU0", "TCU1", "TCU2", "TCU3", "TCU4", "TCU5", "TCU6", "TCU7",
> +};
> +
> +static int __init ingenic_tcu_setup_cevt(struct device_node *np,
> + struct ingenic_tcu *tcu, unsigned int idx)
> +{
> + struct ingenic_tcu_channel *channel = &tcu->channels[idx];
> + struct ingenic_clock_event_device *jzcevt;
> + unsigned long rate;
> + int err, virq;
> +
> + err = ingenic_tcu_req_channel(channel);
> + if (err)
> + return err;
> +
> + err = ingenic_tcu_reset_channel(np, channel);
> + if (err)
> + goto err_out_free_channel;
> +
> + rate = clk_get_rate(channel->clk);
> + if (!rate) {
> + err = -EINVAL;
> + goto err_out_free_channel;
> + }
> +
> + jzcevt = kzalloc(sizeof(*jzcevt), GFP_KERNEL);
> + if (!jzcevt) {
> + err = -ENOMEM;
> + goto err_out_free_channel;
> + }
> +
> + virq = irq_of_parse_and_map(np, idx);
> + if (!virq) {
> + err = -EINVAL;
> + goto err_out_kfree_jzcevt;
> + }
> +
> + err = request_irq(virq, ingenic_tcu_cevt_cb, IRQF_TIMER,
> + ingenic_tcu_timer_names[idx], &jzcevt->cevt);
> + if (err)
> + goto err_out_irq_dispose_mapping;
> +
> + jzcevt->channel = channel;
> + snprintf(jzcevt->name, sizeof(jzcevt->name), "ingenic-tcu-chan%u",
> + channel->idx);
> +
> + jzcevt->cevt.cpumask = cpumask_of(smp_processor_id());
> + jzcevt->cevt.features = CLOCK_EVT_FEAT_ONESHOT;
> + jzcevt->cevt.name = jzcevt->name;
> + jzcevt->cevt.rating = 200;
> + jzcevt->cevt.set_state_shutdown = ingenic_tcu_cevt_set_state_shutdown;
> + jzcevt->cevt.set_next_event = ingenic_tcu_cevt_set_next;
> +
> + clockevents_config_and_register(&jzcevt->cevt, rate, 10, (1 << 16) - 1);
> +
> + return 0;
> +
> +err_out_irq_dispose_mapping:
> + irq_dispose_mapping(virq);
> +err_out_kfree_jzcevt:
> + kfree(jzcevt);
> +err_out_free_channel:
> + ingenic_tcu_free_channel(channel);
> + return err;
> +}
> +
> +static int __init ingenic_tcu_init(struct device_node *np)
> +{
> + unsigned long available_channels = GENMASK(NUM_CHANNELS - 1, 0);
> + struct device_node *node, *pwm_driver_node;
> + struct ingenic_tcu *tcu;
> + unsigned int i, channel;
> + int err;
> + u32 val;
> +
> + /* Parse the devicetree for clients of the TCU PWM driver;
> + * every TCU channel not requested for PWM will be used as
> + * a timer.
> + */
> + for_each_node_with_property(node, "pwms") {
> + /* Get the PWM channel ID (field 1 of the "pwms" node) */
> + err = of_property_read_u32_index(node, "pwms", 1, &val);
> + if (!err && val >= NUM_CHANNELS)
> + err = -EINVAL;
> + if (err) {
> + pr_err("timer-ingenic: Unable to parse PWM nodes!");
> + break;
> + }
> +
> + /* Get the PWM driver node (field 0 of the "pwms" node) */
> + pwm_driver_node = of_parse_phandle(node, "pwms", 0);
> + if (!pwm_driver_node) {
> + pr_err("timer-ingenic: Unable to find PWM driver node");
> + break;
> + }
> +
> + /* Verify that the node we found is for the TCU PWM driver,
> + * by checking that this driver and the PWM driver passed
> + * as phandle share the same parent (the "ingenic,tcu"
> + * compatible MFD/syscon node).
> + */
> + if (pwm_driver_node->parent != np->parent)
> + continue;
> +
> + pr_info("timer-ingenic: Reserving channel %u for PWM", val);
> + available_channels &= ~BIT(val);
> + }
> +
> + tcu = kzalloc(sizeof(*tcu), GFP_KERNEL);
> + if (!tcu)
> + return -ENOMEM;
> +
> + tcu->map = syscon_node_to_regmap(np->parent);
> + if (IS_ERR(tcu->map)) {
> + err = PTR_ERR(tcu->map);
> + kfree(tcu);
> + return err;
> + }
> +
> + for (i = 0; i < NUM_CHANNELS; i++)
> + tcu->channels[i].idx = i;
I'm pretty sure you can do better thaningenic_tcu_setup that :)
> + for_each_set_bit(channel, &available_channels, NUM_CHANNELS) {
> + err = _cevt(np, tcu, channel);
> + if (err) {
> + pr_warn("timer-ingenic: Unable to init TCU channel %u: %i",
> + channel, err);
> + continue;
> + }
> + }
> +
> + return 0;
> +}
> +
> +/* We only probe via devicetree, no need for a platform driver */
> +CLOCKSOURCE_OF_DECLARE(jz4740_tcu, "ingenic,jz4740-tcu", ingenic_tcu_init);
> +CLOCKSOURCE_OF_DECLARE(jz4770_tcu, "ingenic,jz4770-tcu", ingenic_tcu_init);
> +CLOCKSOURCE_OF_DECLARE(jz4780_tcu, "ingenic,jz4780-tcu", ingenic_tcu_init);
s/CLOCKSOURCE_OF_DECLARE/TIMER_OF_DECLARE/
>
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v5 3/5] i2c: i2c-qcom-geni: Add bus driver for the Qualcomm GENI I2C controller
From: Sagar Dharia @ 2018-03-24 21:54 UTC (permalink / raw)
To: Doug Anderson, Karthikeyan Ramasubramanian
Cc: Jonathan Corbet, Andy Gross, David Brown, Rob Herring,
Mark Rutland, Wolfram Sang, linux-doc, linux-arm-msm, devicetree,
linux-i2c, evgreen, acourbot, swboyd, Girish Mahadevan
In-Reply-To: <CAD=FV=VGcttQ6H1-Hnqj+Jrk+eaE52bOna6NojG5DoQ--A5aEg@mail.gmail.com>
Hi Doug
On 3/23/2018 5:34 PM, Doug Anderson wrote:
> Hi,
>
> On Fri, Mar 23, 2018 at 1:20 PM, Karthikeyan Ramasubramanian
> <kramasub@codeaurora.org> wrote:
>> This bus driver supports the GENI based i2c hardware controller in the
>> Qualcomm SOCs. The Qualcomm Generic Interface (GENI) is a programmable
>> module supporting a wide range of serial interfaces including I2C. The
>> driver supports FIFO mode and DMA mode of transfer and switches modes
>> dynamically depending on the size of the transfer.
>>
>> Signed-off-by: Karthikeyan Ramasubramanian <kramasub@codeaurora.org>
>> Signed-off-by: Sagar Dharia <sdharia@codeaurora.org>
>> Signed-off-by: Girish Mahadevan <girishm@codeaurora.org>
>> ---
>> drivers/i2c/busses/Kconfig | 13 +
>> drivers/i2c/busses/Makefile | 1 +
>> drivers/i2c/busses/i2c-qcom-geni.c | 650 +++++++++++++++++++++++++++++++++++++
>> 3 files changed, 664 insertions(+)
>
> [...]
>
>> +/*
>> + * Hardware uses the underlying formula to calculate time periods of
>> + * SCL clock cycle. Firmware uses some additional cycles excluded from the
>> + * below formula and it is confirmed that the time periods are within
>> + * specification limits.
>
> I was hoping for more than just "oh, and there's a fudge factor", but
> I guess this is the best I'm going to get?
>
According to our HW/FW team:
"We use over sampling in our FW and we use 1-2 NOPE extra in some cases.
this is why we can’t give a exact formula."
>
>> +static int geni_i2c_probe(struct platform_device *pdev)
>> +{
>> + struct geni_i2c_dev *gi2c;
>> + struct resource *res;
>> + u32 proto, tx_depth;
>> + int ret;
>> +
>> + gi2c = devm_kzalloc(&pdev->dev, sizeof(*gi2c), GFP_KERNEL);
>> + if (!gi2c)
>> + return -ENOMEM;
>> +
>> + gi2c->se.dev = &pdev->dev;
>> + gi2c->se.wrapper = dev_get_drvdata(pdev->dev.parent);
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + gi2c->se.base = devm_ioremap_resource(&pdev->dev, res);
>> + if (IS_ERR(gi2c->se.base))
>> + return PTR_ERR(gi2c->se.base);
>> +
>> + gi2c->se.clk = devm_clk_get(&pdev->dev, "se");
>> + if (IS_ERR(gi2c->se.clk)) {
>> + ret = PTR_ERR(gi2c->se.clk);
>> + dev_err(&pdev->dev, "Err getting SE Core clk %d\n", ret);
>> + return ret;
>> + }
>> +
>> + ret = device_property_read_u32(&pdev->dev, "clock-frequency",
>> + &gi2c->clk_freq_out);
>> + if (ret) {
>> + /* Clock frequency not specified, so default to 100kHz. */
>> + dev_info(&pdev->dev,
>> + "Bus frequency not specified, default to 100kHz.\n");
>
> If you happen to spin again, can you remove the comment since it's
> obvious from the string in the print? It looks a lot like this code:
>
> /* Print hello, world */
> printf("hello, world\n");
>
>
> In any case, that's a pretty minor nit, so I'll add:
>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
>
> ...assuming that the bindings and "geni" code get Acked / landed
> somewhere. Ideally let's not land this before the geni code lands
> since if the geni API changes for some reason it'll cause us grief.
>
Sure, thanks a lot for reviewing the patches!
-Sagar
>
> -Doug
>
--
Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3 01/11] i2c: Export of_i2c_get_board_info()
From: Wolfram Sang @ 2018-03-24 22:35 UTC (permalink / raw)
To: Boris Brezillon
Cc: linux-i2c, Jonathan Corbet, linux-doc, Greg Kroah-Hartman,
Arnd Bergmann, Przemyslaw Sroka, Arkadiusz Golec, Alan Douglas,
Bartosz Folta, Damian Kos, Alicja Jurasik-Urbaniak,
Cyprian Wronka, Suresh Punnoose, Rafal Ciepiela, Thomas Petazzoni,
Nishanth Menon, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, devicetree, linux-kernel, Vitor Soares,
Geert Uytterhoeven, Linus Walleij, Xiang Lin, linux-gpio,
Boris Brezillon
In-Reply-To: <20180323110020.19080-2-boris.brezillon@bootlin.com>
[-- Attachment #1: Type: text/plain, Size: 230 bytes --]
Hi Boris,
> - rebase on v4.15-rc1
This code has changed a little meanwhile. Please check my for-next
branch. Some changes are identical, some similar.
> - info.archdata = &dev_ad;
Why did you drop this?
Regards,
Wolfram
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v3 01/11] i2c: Export of_i2c_get_board_info()
From: Wolfram Sang @ 2018-03-24 22:38 UTC (permalink / raw)
To: Boris Brezillon
Cc: linux-i2c, Jonathan Corbet, linux-doc, Greg Kroah-Hartman,
Arnd Bergmann, Przemyslaw Sroka, Arkadiusz Golec, Alan Douglas,
Bartosz Folta, Damian Kos, Alicja Jurasik-Urbaniak,
Cyprian Wronka, Suresh Punnoose, Rafal Ciepiela, Thomas Petazzoni,
Nishanth Menon, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, devicetree, linux-kernel, Vitor Soares,
Geert Uytterhoeven, Linus Walleij, Xiang Lin, linux-gpio,
Boris Brezillon
In-Reply-To: <20180324223518.g5jsi5zvoxa75psy@ninjato>
[-- Attachment #1: Type: text/plain, Size: 132 bytes --]
> > - info.archdata = &dev_ad;
>
> Why did you drop this?
If the removal is safe, it should be a seperate patch, I mean.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v3 01/11] i2c: Export of_i2c_get_board_info()
From: Boris Brezillon @ 2018-03-25 10:19 UTC (permalink / raw)
To: Wolfram Sang
Cc: linux-i2c, Jonathan Corbet, linux-doc, Greg Kroah-Hartman,
Arnd Bergmann, Przemyslaw Sroka, Arkadiusz Golec, Alan Douglas,
Bartosz Folta, Damian Kos, Alicja Jurasik-Urbaniak,
Cyprian Wronka, Suresh Punnoose, Rafal Ciepiela, Thomas Petazzoni,
Nishanth Menon, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, devicetree, linux-kernel, Vitor Soares,
Geert Uytterhoeven, Linus Walleij, Xiang Lin, linux-gpio,
Boris Brezillon
In-Reply-To: <20180324223518.g5jsi5zvoxa75psy@ninjato>
Hi Wolfram,
On Sat, 24 Mar 2018 23:35:18 +0100
Wolfram Sang <wsa@the-dreams.de> wrote:
> Hi Boris,
>
> > - rebase on v4.15-rc1
>
> This code has changed a little meanwhile. Please check my for-next
> branch. Some changes are identical, some similar.
Actually it was rebased on top of v4.16-rc1, but I'll check if this
patch applies correctly on top of your for-next branch.
>
> > - info.archdata = &dev_ad;
>
> Why did you drop this?
Well, this line could have been moved in of_i2c_register_device(), but
I realized dev_ad was zero initialized without any further
modification, and leaving info->archdata to NULL has the exact same
effect (see [1]).
Regards,
Boris
[1]https://elixir.bootlin.com/linux/v4.16-rc6/source/drivers/i2c/i2c-core-base.c#L711
--
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3 01/11] i2c: Export of_i2c_get_board_info()
From: Boris Brezillon @ 2018-03-25 10:21 UTC (permalink / raw)
To: Wolfram Sang
Cc: linux-i2c, Jonathan Corbet, linux-doc, Greg Kroah-Hartman,
Arnd Bergmann, Przemyslaw Sroka, Arkadiusz Golec, Alan Douglas,
Bartosz Folta, Damian Kos, Alicja Jurasik-Urbaniak,
Cyprian Wronka, Suresh Punnoose, Rafal Ciepiela, Thomas Petazzoni,
Nishanth Menon, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, devicetree, linux-kernel, Vitor Soares,
Geert Uytterhoeven, Linus Walleij, Xiang Lin, linux-gpio,
Boris Brezillon
In-Reply-To: <20180324223832.7wvhghw77ech4orn@ninjato>
On Sat, 24 Mar 2018 23:38:32 +0100
Wolfram Sang <wsa@the-dreams.de> wrote:
> > > - info.archdata = &dev_ad;
> >
> > Why did you drop this?
>
> If the removal is safe, it should be a seperate patch, I mean.
>
Sure, I'll move that in a separate patch. Actually, I had a closer look
and it seems the only user of info->archdata is i2c-core-of.c, so we
can even remove the info->archdata field.
--
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH] syscalls: define and explain goal to not call syscalls in the kernel
From: Dominik Brodowski @ 2018-03-25 16:25 UTC (permalink / raw)
To: linux-kernel
Cc: linux-doc, Jonathan Corbet, viro, x86, torvalds, mingo, tglx,
luto
The syscall entry points to the kernel defined by SYSCALL_DEFINEx()
and COMPAT_SYSCALL_DEFINEx() should only be called from userspace
through kernel entry points, but not from the kernel itself. This
will allow cleanups and optimizations to the entry paths *and* to
the parts of the kernel code which currently need to pretend to be
userspace in order to make use of syscalls.
Signed-off-by: Dominik Brodowski <linux@dominikbrodowski.net>
---
As there have been multiple inquiries on the rationale of my patchsets
removing in-kernel calls to sys_xyzzy(), here is an updated patch 01/NN
which I will push upstream for v4.17-rc1. I will also include a reference
to this mail (and therefore to the explanation below) in all related
patches of the series. Any improvements, hints, suggestions, spelling
fixes, and/or objections?
Thanks,
Dominik
diff --git a/Documentation/process/adding-syscalls.rst b/Documentation/process/adding-syscalls.rst
index 8cc25a06f353..556613744556 100644
--- a/Documentation/process/adding-syscalls.rst
+++ b/Documentation/process/adding-syscalls.rst
@@ -487,6 +487,38 @@ patchset, for the convenience of reviewers.
The man page should be cc'ed to linux-man@vger.kernel.org
For more details, see https://www.kernel.org/doc/man-pages/patches.html
+
+Do not call System Calls in the Kernel
+--------------------------------------
+
+System calls are, as stated above, interaction points between userspace and
+the kernel. Therefore, system call functions such as ``sys_xyzzy()`` or
+``compat_sys_xyzzy()`` should only be called from userspace via the syscall
+table, but not from elsewhere in the kernel. If the syscall functionality is
+useful to be used within the kernel, needs to be shared between an old and a
+new syscall, or needs to be shared between a syscall and its compatibility
+variant, it should be implemented by means of a "helper" function (such as
+``kern_xyzzy()``). This kernel function may then be called within the
+syscall stub (``sys_xyzzy()``), the compatibility syscall stub
+(``compat_sys_xyzzy()``), and/or other kernel code.
+
+At least on 64-bit x86, it will be a hard requirement from v4.17 onwards to not
+call system call functions in the kernel. It uses a different calling
+convention for system calls where ``struct pt_regs`` is decoded on-the-fly in a
+syscall wrapper which then hands processing over to the actual syscall function.
+This means that only those parameters which are actually needed for a specific
+syscall are passed on during syscall entry, instead of filling in six CPU
+registers with random user space content all the time (which may cause serious
+trouble down the call chain).
+
+Moreover, rules on how data may be accessed may differ between kernel data and
+user data. This is another reason why calling ``sys_xyzzy()`` is generally a
+bad idea.
+
+Exceptions to this rule are only allowed in architecture-specific overrides,
+architecture-specific compatibility wrappers, or other code in arch/.
+
+
References and Sources
----------------------
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index a78186d826d7..0526286a0314 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -941,4 +941,11 @@ asmlinkage long sys_pkey_free(int pkey);
asmlinkage long sys_statx(int dfd, const char __user *path, unsigned flags,
unsigned mask, struct statx __user *buffer);
+
+/*
+ * Kernel code should not call syscalls (i.e., sys_xyzyyz()) directly.
+ * Instead, use one of the functions which work equivalently, such as
+ * the ksys_xyzyyz() functions prototyped below.
+ */
+
#endif
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH 2/2] perf: riscv: Add Document for Future Porting Guide
From: Alan Kao @ 2018-03-26 7:57 UTC (permalink / raw)
To: Palmer Dabbelt, Albert Ou, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Alexander Shishkin, Jiri Olsa,
Namhyung Kim, Alex Solomatnikov, Jonathan Corbet, linux-riscv,
linux-doc, linux-kernel
Cc: Alan Kao, Nick Hu, Greentime Hu
In-Reply-To: <1522051075-6442-1-git-send-email-alankao@andestech.com>
Cc: Nick Hu <nickhu@andestech.com>
Cc: Greentime Hu <greentime@andestech.com>
Signed-off-by: Alan Kao <alankao@andestech.com>
---
Documentation/riscv/pmu.txt | 250 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 250 insertions(+)
create mode 100644 Documentation/riscv/pmu.txt
diff --git a/Documentation/riscv/pmu.txt b/Documentation/riscv/pmu.txt
new file mode 100644
index 000000000000..496f00476560
--- /dev/null
+++ b/Documentation/riscv/pmu.txt
@@ -0,0 +1,250 @@
+Supporting PMUs on RISC-V platforms
+==========================================
+Alan Kao <alankao@andestech.com>, Mar 2018
+
+Introduction
+------------
+
+As of this writing, perf_event-related features mentioned in The RISC-V ISA
+Privileged Version 1.10 are as follows:
+(please check the manual for more details)
+
+* [m|s]counteren
+* mcycle[h], cycle[h]
+* minstret[h], instret[h]
+* mhpeventx, mhpcounterx[h]
+
+With such function set only, porting perf would require a lot of work, due to
+the lack of the following general architectural performance monitoring features:
+
+* Enabling/Disabling counters
+ Counters are just free-running all the time in our case.
+* Interrupt caused by counter overflow
+ No such design in the spec.
+* Interrupt indicator
+ It is not possible to have many interrupt ports for all counters, so an
+ interrupt indicator is required for software to tell which counter has
+ just overflowed.
+* Writing to counters
+ There will be an SBI to support this since the kernel cannot modify the
+ counters [1]. Alternatively, some vendor considers to implement
+ hardware-extension for M-S-U model machines to write counters directly.
+
+This document aims to provide developers a quick guide on supporting their
+PMUs in the kernel. The following sections briefly explain perf' mechanism
+and todos.
+
+You may check previous discussions here [1][2]. Also, it might be helpful
+to check the appendix for related kernel structures.
+
+
+1. Initialization
+-----------------
+
+*riscv_pmu* is a global pointer of type *struct riscv_pmu*, which contains
+various methods according to perf's internal convention and PMU-specific
+parameters. One should declare such instance to represent the PMU. By default,
+*riscv_pmu* points to a constant structure *riscv_base_pmu*, which has very
+basic support to a baseline QEMU model.
+
+Then he/she can either assign the instance's pointer to *riscv_pmu* so that
+the minimal and already-implemented logic can be leveraged, or invent his/her
+own *riscv_init_platform_pmu* implementation.
+
+In other words, existing sources of *riscv_base_pmu* merely provide a
+reference implementation. Developers can flexibly decide how many parts they
+can leverage, and in the most extreme case, they can customize every function
+according to their needs.
+
+
+2. Event Initialization
+-----------------------
+
+When a user launches a perf command to monitor some events, it is first
+interpreted by the userspace perf tool into multiple *perf_event_open*
+system calls, and then each of them calls to the body of *event_init*
+member function that was assigned in the previous step. In *riscv_base_pmu*'s
+case, it is *riscv_event_init*.
+
+The main purpose of this function is to translate the event provided by user
+into bitmap, so that HW-related control registers or counters can directly be
+manipulated. The translation is based on the mappings and methods provided in
+*riscv_pmu*.
+
+Note that some features can be done in this stage as well:
+
+(1) interrupt setting, which is stated in the next section;
+(2) privilege level setting (user space only, kernel space only, both);
+(3) destructor setting. Normally it is sufficient to apply *riscv_destroy_event*;
+(4) tweaks for non-sampling events, which will be utilized by functions such as
+*perf_adjust_period*, usually something like the follows:
+
+if (!is_sampling_event(event)) {
+ hwc->sample_period = x86_pmu.max_period;
+ hwc->last_period = hwc->sample_period;
+ local64_set(&hwc->period_left, hwc->sample_period);
+}
+
+In the case of *riscv_base_pmu*, only (3) is provided for now.
+
+
+3. Interrupt
+------------
+
+3.1. Interrupt Initialization
+
+This often occurs at the beginning of the *event_init* method. In common
+practice, this should be a code segment like
+
+int x86_reserve_hardware(void)
+{
+ int err = 0;
+
+ if (!atomic_inc_not_zero(&pmc_refcount)) {
+ mutex_lock(&pmc_reserve_mutex);
+ if (atomic_read(&pmc_refcount) == 0) {
+ if (!reserve_pmc_hardware())
+ err = -EBUSY;
+ else
+ reserve_ds_buffers();
+ }
+ if (!err)
+ atomic_inc(&pmc_refcount);
+ mutex_unlock(&pmc_reserve_mutex);
+ }
+
+ return err;
+}
+
+And the magic is in *reserve_pmc_hardware*, which usually does atomic
+operations to make implemented IRQ accessible from some global function pointer.
+*release_pmc_hardware* serves the opposite purpose, and it is used in event
+destructors mentioned in previous section.
+
+(Note: From the implementations in all the architectures, the *reserve/release*
+pair are always IRQ settings, so the *pmc_hardware* seems somehow misleading.
+It does NOT deal with the binding between an event and a physical counter,
+which will be introduced in the next section.)
+
+3.2. IRQ Structure
+
+Basically, a IRQ runs the following pseudo code:
+
+for each hardware counter that triggered this overflow
+
+ get the event of this counter
+
+ // following two steps are defined as *read()*,
+ // check the section Reading/Writing Counters for details.
+ count the delta value since previous interrupt
+ update the event->count (# event occurs) by adding delta, and
+ event->hw.period_left by subtracting delta
+
+ if the event overflows
+ sample data
+ set the counter appropriately for the next overflow
+
+ if the event overflows again
+ too frequently, throttle this event
+ fi
+ fi
+
+end for
+
+However as of this writing, none of the RISC-V implementations have designed an
+interrupt for perf, so the details are to be completed in the future.
+
+4. Reading/Writing Counters
+---------------------------
+
+They seem symmetric but perf treats them quite differently. For reading, there
+is a *read* interface in *struct pmu*, but it serves more than just reading.
+According to the context, the *read* function not only read the content of the
+counter (event->count), but also update the left period to the next interrupt
+(event->hw.period_left).
+
+But the core of perf does not need direct write to counters. Writing counters
+hides behind the abstraction of 1) *pmu->start*, literally start counting so one
+has to set the counter to a good value for the next interrupt; 2) inside the IRQ
+it should set the counter with the same reason.
+
+Reading is not a problem in RISC-V but writing would need some effort, since
+counters are not allowed to be written by S-mode.
+
+
+5. add()/del()/start()/stop()
+-----------------------------
+
+Basic idea: add()/del() adds/deletes events to/from a PMU, and start()/stop()
+starts/stop the counter of some event in the PMU. All of them take the same
+arguments: *struct perf_event *event* and *int flag*.
+
+Consider perf as a state machine, then you will find that these functions serve
+as the state transition process between those states.
+Three states (event->hw.state) are defined:
+
+* PERF_HES_STOPPED: the counter is stopped
+* PERF_HES_UPTODATE: the event->count is up-to-date
+* PERF_HES_ARCH: arch-dependent usage ... we don't need this for now
+
+A normal flow of these state transitions are as follows:
+
+* A user launches a perf event, resulting in calling to *event_init*.
+* When being context-switched in, *add* is called by the perf core, with flag
+ PERF_EF_START, which mean that the event should be started after it is added.
+ In this stage, an general event is binded to a physical counter, if any.
+ The state changes to PERF_HES_STOPPED and PERF_HES_UPTODATE, because it is now
+ stopped, and the (software) event count does not need updating.
+** *start* is then called, and the counter is enabled.
+ With flag PERF_EF_RELOAD, it write the counter to an appropriate value (check
+ previous section for detail).
+ No writing is made if the flag does not contain PERF_EF_RELOAD.
+ The state now is reset to none, because it is neither stopped nor update
+ (the counting already starts)
+* When being context-switched out, *del* is called. It then checkout all the
+ events in the PMU and call *stop* to update their counts.
+** *stop* is called by *del*
+ and the perf core with flag PERF_EF_UPDATE, and it often shares the same
+ subroutine as *read* with the same logic.
+ The state changes to PERF_HES_STOPPED and PERF_HES_UPTODATE, again.
+
+** Life cycles of these two pairs: *add* and *del* are called repeatedly as
+ tasks switch in-and-out; *start* and *stop* is also called when the perf core
+ needs a quick stop-and-start, for instance, when the interrupt period is being
+ adjusted.
+
+Current implementation is sufficient for now and can be easily extend to
+features in the future.
+
+A. Related Structures
+---------------------
+
+* struct pmu: include/linux/perf_events.h
+* struct riscv_pmu: arch/riscv/include/asm/perf_events.h
+
+ Both structures are designed to be read-only.
+
+ *struct pmu* defines some function pointer interfaces, and most of them take
+*struct perf_event* as a main argument, dealing with perf events according to
+perf's internal state machine (check kernel/events/core.c for details).
+
+ *struct riscv_pmu* defines PMU-specific parameters. The naming follows the
+convention of all other architectures.
+
+* struct perf_event: include/linux/perf_events.h
+* struct hw_perf_event
+
+ The generic structure that represents perf events, and the hardware-related
+details.
+
+* struct riscv_hw_events: arch/riscv/include/asm/perf_events.h
+
+ The structure that holds the status of events, has two fixed members:
+the number of events and the array of the events.
+
+References
+----------
+
+[1] https://github.com/riscv/riscv-linux/pull/124
+[2] https://groups.google.com/a/groups.riscv.org/forum/#!topic/sw-dev/f19TmCNP6yA
+
--
2.16.2
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH 0/2] perf: riscv: Preliminary Perf Event Support on RISC-V
From: Alan Kao @ 2018-03-26 7:57 UTC (permalink / raw)
To: Palmer Dabbelt, Albert Ou, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Alexander Shishkin, Jiri Olsa,
Namhyung Kim, Alex Solomatnikov, Jonathan Corbet, linux-riscv,
linux-doc, linux-kernel
Cc: Alan Kao
This implements the baseline PMU for RISC-V platforms.
To ease the future PMU portings, a guide is also written, containing
perf concepts, arch porting practices and some hints.
Alan Kao (2):
perf: riscv: preliminary RISC-V support
perf: riscv: Add Document for Future Porting Guide
Documentation/riscv/pmu.txt | 250 +++++++++++++++++++
arch/riscv/Kconfig | 12 +
arch/riscv/include/asm/perf_event.h | 76 +++++-
arch/riscv/kernel/Makefile | 1 +
arch/riscv/kernel/perf_event.c | 469 ++++++++++++++++++++++++++++++++++++
5 files changed, 804 insertions(+), 4 deletions(-)
create mode 100644 Documentation/riscv/pmu.txt
create mode 100644 arch/riscv/kernel/perf_event.c
--
2.16.2
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH 1/2] perf: riscv: preliminary RISC-V support
From: Alan Kao @ 2018-03-26 7:57 UTC (permalink / raw)
To: Palmer Dabbelt, Albert Ou, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Alexander Shishkin, Jiri Olsa,
Namhyung Kim, Alex Solomatnikov, Jonathan Corbet, linux-riscv,
linux-doc, linux-kernel
Cc: Alan Kao, Nick Hu, Greentime Hu
In-Reply-To: <1522051075-6442-1-git-send-email-alankao@andestech.com>
This patch provide a basic PMU, riscv_base_pmu, which supports two
general hardware event, instructions and cycles. Furthermore, this
PMU serves as a reference implementation to ease the portings in
the future.
riscv_base_pmu should be able to run on any RISC-V machine that
conforms to the Priv-Spec. Note that the latest qemu model hasn't
fully support a proper behavior of Priv-Spec 1.10 yet, but work
around should be easy with very small fixes. Please check
https://github.com/riscv/riscv-qemu/pull/115 for future updates.
Cc: Nick Hu <nickhu@andestech.com>
Cc: Greentime Hu <greentime@andestech.com>
Signed-off-by: Alan Kao <alankao@andestech.com>
---
arch/riscv/Kconfig | 12 +
arch/riscv/include/asm/perf_event.h | 76 +++++-
arch/riscv/kernel/Makefile | 1 +
arch/riscv/kernel/perf_event.c | 469 ++++++++++++++++++++++++++++++++++++
4 files changed, 554 insertions(+), 4 deletions(-)
create mode 100644 arch/riscv/kernel/perf_event.c
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 310b9a5d6737..dd4aecfb5265 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -195,6 +195,18 @@ config RISCV_ISA_C
config RISCV_ISA_A
def_bool y
+menu "PMU type"
+ depends on PERF_EVENTS
+
+config RISCV_BASE_PMU
+ bool "Base Performance Monitoring Unit"
+ def_bool y
+ help
+ A base PMU that serves as a reference implementation and has limited
+ feature of perf.
+
+endmenu
+
endmenu
menu "Kernel type"
diff --git a/arch/riscv/include/asm/perf_event.h b/arch/riscv/include/asm/perf_event.h
index e13d2ff29e83..98e2efb02d25 100644
--- a/arch/riscv/include/asm/perf_event.h
+++ b/arch/riscv/include/asm/perf_event.h
@@ -1,13 +1,81 @@
+/* SPDX-License-Identifier: GPL-2.0 */
/*
* Copyright (C) 2018 SiFive
+ * Copyright (C) 2018 Andes Technology Corporation
*
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public Licence
- * as published by the Free Software Foundation; either version
- * 2 of the Licence, or (at your option) any later version.
*/
#ifndef _ASM_RISCV_PERF_EVENT_H
#define _ASM_RISCV_PERF_EVENT_H
+#include <linux/perf_event.h>
+#include <linux/ptrace.h>
+
+#define RISCV_BASE_COUNTERS 2
+
+/*
+ * The RISCV_MAX_COUNTERS parameter should be specified.
+ */
+
+#ifdef CONFIG_RISCV_BASE_PMU
+#define RISCV_MAX_COUNTERS 2
+#endif
+
+#ifndef RISCV_MAX_COUNTERS
+#error "Please provide a valid RISCV_MAX_COUNTERS for the PMU."
+#endif
+
+/*
+ * These are the indexes of bits in counteren register *minus* 1,
+ * except for cycle. It would be coherent if it can directly mapped
+ * to counteren bit definition, but there is a *time* register at
+ * counteren[1]. Per-cpu structure is scarce resource here.
+ *
+ * According to the spec, an implementation can support counter up to
+ * mhpmcounter31, but many high-end processors has at most 6 general
+ * PMCs, we give the definition to MHPMCOUNTER8 here.
+ */
+#define RISCV_PMU_CYCLE 0
+#define RISCV_PMU_INSTRET 1
+#define RISCV_PMU_MHPMCOUNTER3 2
+#define RISCV_PMU_MHPMCOUNTER4 3
+#define RISCV_PMU_MHPMCOUNTER5 4
+#define RISCV_PMU_MHPMCOUNTER6 5
+#define RISCV_PMU_MHPMCOUNTER7 6
+#define RISCV_PMU_MHPMCOUNTER8 7
+
+#define RISCV_OP_UNSUPP (-EOPNOTSUPP)
+
+struct cpu_hw_events {
+ /* # currently enabled events*/
+ int n_events;
+ /* currently enabled events */
+ struct perf_event *events[RISCV_MAX_COUNTERS];
+ /* vendor-defined PMU data */
+ void *platform;
+};
+
+struct riscv_pmu {
+ struct pmu *pmu;
+
+ /* generic hw/cache events table */
+ const int *hw_events;
+ const int (*cache_events)[PERF_COUNT_HW_CACHE_MAX]
+ [PERF_COUNT_HW_CACHE_OP_MAX]
+ [PERF_COUNT_HW_CACHE_RESULT_MAX];
+ /* method used to map hw/cache events */
+ int (*map_hw_event)(u64 config);
+ int (*map_cache_event)(u64 config);
+
+ /* max generic hw events in map */
+ int max_events;
+ /* number total counters, 2(base) + x(general) */
+ int num_counters;
+ /* the width of the counter */
+ int counter_width;
+
+ /* vendor-defined PMU features */
+ void *platform;
+};
+
#endif /* _ASM_RISCV_PERF_EVENT_H */
diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile
index 196f62ffc428..849c38d9105f 100644
--- a/arch/riscv/kernel/Makefile
+++ b/arch/riscv/kernel/Makefile
@@ -36,5 +36,6 @@ obj-$(CONFIG_SMP) += smp.o
obj-$(CONFIG_MODULES) += module.o
obj-$(CONFIG_FUNCTION_TRACER) += mcount.o
obj-$(CONFIG_FUNCTION_GRAPH_TRACER) += ftrace.o
+obj-$(CONFIG_PERF_EVENTS) += perf_event.o
clean:
diff --git a/arch/riscv/kernel/perf_event.c b/arch/riscv/kernel/perf_event.c
new file mode 100644
index 000000000000..b78cb486683b
--- /dev/null
+++ b/arch/riscv/kernel/perf_event.c
@@ -0,0 +1,469 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2008 Thomas Gleixner <tglx@linutronix.de>
+ * Copyright (C) 2008-2009 Red Hat, Inc., Ingo Molnar
+ * Copyright (C) 2009 Jaswinder Singh Rajput
+ * Copyright (C) 2009 Advanced Micro Devices, Inc., Robert Richter
+ * Copyright (C) 2008-2009 Red Hat, Inc., Peter Zijlstra
+ * Copyright (C) 2009 Intel Corporation, <markus.t.metzger@intel.com>
+ * Copyright (C) 2009 Google, Inc., Stephane Eranian
+ * Copyright 2014 Tilera Corporation. All Rights Reserved.
+ * Copyright (C) 2018 Andes Technology Corporation
+ *
+ * Perf_events support for RISC-V platforms.
+ *
+ * Since the spec. (as of now, Priv-Spec 1.10) does not provide enough
+ * functionality for perf event to fully work, this file provides
+ * the very basic framework only.
+ *
+ * For platform portings, please check Documentations/riscv/pmu.txt.
+ *
+ * The Copyright line includes x86 and tile ones.
+ */
+
+#include <linux/kprobes.h>
+#include <linux/kernel.h>
+#include <linux/kdebug.h>
+#include <linux/mutex.h>
+#include <linux/bitmap.h>
+#include <linux/irq.h>
+#include <linux/interrupt.h>
+#include <linux/perf_event.h>
+#include <linux/atomic.h>
+#include <asm/perf_event.h>
+
+static const struct riscv_pmu *riscv_pmu __read_mostly;
+static DEFINE_PER_CPU(struct cpu_hw_events, cpu_hw_events);
+
+/*
+ * Hardware & cache maps and their methods
+ */
+
+static const int riscv_hw_event_map[] = {
+ [PERF_COUNT_HW_CPU_CYCLES] = RISCV_PMU_CYCLE,
+ [PERF_COUNT_HW_INSTRUCTIONS] = RISCV_PMU_INSTRET,
+ [PERF_COUNT_HW_CACHE_REFERENCES] = RISCV_OP_UNSUPP,
+ [PERF_COUNT_HW_CACHE_MISSES] = RISCV_OP_UNSUPP,
+ [PERF_COUNT_HW_BRANCH_INSTRUCTIONS] = RISCV_OP_UNSUPP,
+ [PERF_COUNT_HW_BRANCH_MISSES] = RISCV_OP_UNSUPP,
+ [PERF_COUNT_HW_BUS_CYCLES] = RISCV_OP_UNSUPP,
+};
+
+#define C(x) PERF_COUNT_HW_CACHE_##x
+static const int riscv_cache_event_map[PERF_COUNT_HW_CACHE_MAX]
+[PERF_COUNT_HW_CACHE_OP_MAX]
+[PERF_COUNT_HW_CACHE_RESULT_MAX] = {
+ [C(L1D)] = {
+ [C(OP_READ)] = {
+ [C(RESULT_ACCESS)] = RISCV_OP_UNSUPP,
+ [C(RESULT_MISS)] = RISCV_OP_UNSUPP,
+ },
+ [C(OP_WRITE)] = {
+ [C(RESULT_ACCESS)] = RISCV_OP_UNSUPP,
+ [C(RESULT_MISS)] = RISCV_OP_UNSUPP,
+ },
+ [C(OP_PREFETCH)] = {
+ [C(RESULT_ACCESS)] = RISCV_OP_UNSUPP,
+ [C(RESULT_MISS)] = RISCV_OP_UNSUPP,
+ },
+ },
+ [C(L1I)] = {
+ [C(OP_READ)] = {
+ [C(RESULT_ACCESS)] = RISCV_OP_UNSUPP,
+ [C(RESULT_MISS)] = RISCV_OP_UNSUPP,
+ },
+ [C(OP_WRITE)] = {
+ [C(RESULT_ACCESS)] = RISCV_OP_UNSUPP,
+ [C(RESULT_MISS)] = RISCV_OP_UNSUPP,
+ },
+ [C(OP_PREFETCH)] = {
+ [C(RESULT_ACCESS)] = RISCV_OP_UNSUPP,
+ [C(RESULT_MISS)] = RISCV_OP_UNSUPP,
+ },
+ },
+ [C(LL)] = {
+ [C(OP_READ)] = {
+ [C(RESULT_ACCESS)] = RISCV_OP_UNSUPP,
+ [C(RESULT_MISS)] = RISCV_OP_UNSUPP,
+ },
+ [C(OP_WRITE)] = {
+ [C(RESULT_ACCESS)] = RISCV_OP_UNSUPP,
+ [C(RESULT_MISS)] = RISCV_OP_UNSUPP,
+ },
+ [C(OP_PREFETCH)] = {
+ [C(RESULT_ACCESS)] = RISCV_OP_UNSUPP,
+ [C(RESULT_MISS)] = RISCV_OP_UNSUPP,
+ },
+ },
+ [C(DTLB)] = {
+ [C(OP_READ)] = {
+ [C(RESULT_ACCESS)] = RISCV_OP_UNSUPP,
+ [C(RESULT_MISS)] = RISCV_OP_UNSUPP,
+ },
+ [C(OP_WRITE)] = {
+ [C(RESULT_ACCESS)] = RISCV_OP_UNSUPP,
+ [C(RESULT_MISS)] = RISCV_OP_UNSUPP,
+ },
+ [C(OP_PREFETCH)] = {
+ [C(RESULT_ACCESS)] = RISCV_OP_UNSUPP,
+ [C(RESULT_MISS)] = RISCV_OP_UNSUPP,
+ },
+ },
+ [C(ITLB)] = {
+ [C(OP_READ)] = {
+ [C(RESULT_ACCESS)] = RISCV_OP_UNSUPP,
+ [C(RESULT_MISS)] = RISCV_OP_UNSUPP,
+ },
+ [C(OP_WRITE)] = {
+ [C(RESULT_ACCESS)] = RISCV_OP_UNSUPP,
+ [C(RESULT_MISS)] = RISCV_OP_UNSUPP,
+ },
+ [C(OP_PREFETCH)] = {
+ [C(RESULT_ACCESS)] = RISCV_OP_UNSUPP,
+ [C(RESULT_MISS)] = RISCV_OP_UNSUPP,
+ },
+ },
+ [C(BPU)] = {
+ [C(OP_READ)] = {
+ [C(RESULT_ACCESS)] = RISCV_OP_UNSUPP,
+ [C(RESULT_MISS)] = RISCV_OP_UNSUPP,
+ },
+ [C(OP_WRITE)] = {
+ [C(RESULT_ACCESS)] = RISCV_OP_UNSUPP,
+ [C(RESULT_MISS)] = RISCV_OP_UNSUPP,
+ },
+ [C(OP_PREFETCH)] = {
+ [C(RESULT_ACCESS)] = RISCV_OP_UNSUPP,
+ [C(RESULT_MISS)] = RISCV_OP_UNSUPP,
+ },
+ },
+};
+
+static int riscv_map_hw_event(u64 config)
+{
+ if (config >= riscv_pmu->max_events)
+ return -EINVAL;
+
+ return riscv_pmu->hw_events[config];
+}
+
+int riscv_map_cache_decode(u64 config, unsigned int *type,
+ unsigned int *op, unsigned int *result)
+{
+ return -ENOENT;
+}
+
+static int riscv_map_cache_event(u64 config)
+{
+ unsigned int type, op, result;
+ int err = -ENOENT;
+ int code;
+
+ err = riscv_map_cache_decode(config, &type, &op, &result);
+ if (!riscv_pmu->cache_events || err)
+ return err;
+
+ if (type >= PERF_COUNT_HW_CACHE_MAX ||
+ op >= PERF_COUNT_HW_CACHE_OP_MAX ||
+ result >= PERF_COUNT_HW_CACHE_RESULT_MAX)
+ return -EINVAL;
+
+ code = (*riscv_pmu->cache_events)[type][op][result];
+ if (code == RISCV_OP_UNSUPP)
+ return -EINVAL;
+
+ return code;
+}
+
+/*
+ * Low-level functions: reading/writing counters
+ */
+
+static inline u64 read_counter(int idx)
+{
+ u64 val = 0;
+
+ switch (idx) {
+ case RISCV_PMU_CYCLE:
+ val = csr_read(cycle);
+ break;
+ case RISCV_PMU_INSTRET:
+ val = csr_read(instret);
+ break;
+ default:
+ WARN_ON_ONCE(idx < 0 || idx > RISCV_MAX_COUNTERS);
+ return -EINVAL;
+ }
+
+ return val;
+}
+
+static inline void write_counter(int idx, u64 value)
+{
+ /* currently not supported */
+}
+
+/*
+ * pmu->read: read and update the counter
+ *
+ * Other architectures' implementation often have a xxx_perf_event_update
+ * routine, which can return counter values when called in the IRQ, but
+ * return void when being called by the pmu->read method.
+ */
+static void riscv_pmu_read(struct perf_event *event)
+{
+ struct hw_perf_event *hwc = &event->hw;
+ u64 prev_raw_count, new_raw_count;
+ u64 oldval;
+ int idx = hwc->idx;
+ u64 delta;
+
+ do {
+ prev_raw_count = local64_read(&hwc->prev_count);
+ new_raw_count = read_counter(idx);
+
+ oldval = local64_cmpxchg(&hwc->prev_count, prev_raw_count,
+ new_raw_count);
+ } while (oldval != prev_raw_count);
+
+ /*
+ * delta is the value to update the counter we maintain in the kernel.
+ */
+ delta = (new_raw_count - prev_raw_count) &
+ ((1ULL << riscv_pmu->counter_width) - 1);
+ local64_add(delta, &event->count);
+ /*
+ * Something like local64_sub(delta, &hwc->period_left) here is
+ * needed if there is an interrupt for perf.
+ */
+}
+
+/*
+ * State transition functions:
+ *
+ * stop()/start() & add()/del()
+ */
+
+/*
+ * pmu->stop: stop the counter
+ */
+static void riscv_pmu_stop(struct perf_event *event, int flags)
+{
+ struct hw_perf_event *hwc = &event->hw;
+
+ WARN_ON_ONCE(hwc->state & PERF_HES_STOPPED);
+ hwc->state |= PERF_HES_STOPPED;
+
+ if ((flags & PERF_EF_UPDATE) && !(hwc->state & PERF_HES_UPTODATE)) {
+ riscv_pmu_read(event);
+ hwc->state |= PERF_HES_UPTODATE;
+ }
+}
+
+/*
+ * pmu->start: start the event.
+ */
+static void riscv_pmu_start(struct perf_event *event, int flags)
+{
+ struct hw_perf_event *hwc = &event->hw;
+
+ if (WARN_ON_ONCE(!(event->hw.state & PERF_HES_STOPPED)))
+ return;
+
+ if (flags & PERF_EF_RELOAD) {
+ WARN_ON_ONCE(!(event->hw.state & PERF_HES_UPTODATE));
+
+ /*
+ * Set the counter to the period to the next interrupt here,
+ * if you have any.
+ */
+ }
+
+ hwc->state = 0;
+ perf_event_update_userpage(event);
+
+ /*
+ * Since we cannot write to counters, this serves as an initialization
+ * to the delta-mechanism in pmu->read(); otherwise, the delta would be
+ * wrong when pmu->read is called for the first time.
+ */
+ if (local64_read(&hwc->prev_count) == 0)
+ local64_set(&hwc->prev_count, read_counter(hwc->idx));
+}
+
+/*
+ * pmu->add: add the event to PMU.
+ */
+static int riscv_pmu_add(struct perf_event *event, int flags)
+{
+ struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
+ struct hw_perf_event *hwc = &event->hw;
+
+ if (cpuc->n_events == riscv_pmu->num_counters)
+ return -ENOSPC;
+
+ /*
+ * We don't have general conunters, so no binding-event-to-counter
+ * process here.
+ *
+ * Indexing using hwc->config generally not works, since config may
+ * contain extra information, but here the only info we have in
+ * hwc->config is the event index.
+ */
+ hwc->idx = hwc->config;
+ cpuc->events[hwc->idx] = event;
+ cpuc->n_events++;
+
+ hwc->state = PERF_HES_UPTODATE | PERF_HES_STOPPED;
+
+ if (flags & PERF_EF_START)
+ riscv_pmu_start(event, PERF_EF_RELOAD);
+
+ return 0;
+}
+
+/*
+ * pmu->del: delete the event from PMU.
+ */
+static void riscv_pmu_del(struct perf_event *event, int flags)
+{
+ struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
+ struct hw_perf_event *hwc = &event->hw;
+
+ cpuc->events[hwc->idx] = NULL;
+ cpuc->n_events--;
+ riscv_pmu_stop(event, PERF_EF_UPDATE);
+ perf_event_update_userpage(event);
+}
+
+/*
+ * Interrupt
+ */
+
+static DEFINE_MUTEX(pmc_reserve_mutex);
+typedef void (*perf_irq_t)(void *riscv_perf_irq);
+perf_irq_t perf_irq;
+
+void riscv_pmu_handle_irq(void *riscv_perf_irq)
+{
+}
+
+static perf_irq_t reserve_pmc_hardware(void)
+{
+ perf_irq_t old;
+
+ mutex_lock(&pmc_reserve_mutex);
+ old = perf_irq;
+ perf_irq = &riscv_pmu_handle_irq;
+ mutex_unlock(&pmc_reserve_mutex);
+
+ return old;
+}
+
+void release_pmc_hardware(void)
+{
+ mutex_lock(&pmc_reserve_mutex);
+ perf_irq = NULL;
+ mutex_unlock(&pmc_reserve_mutex);
+}
+
+/*
+ * Event Initialization
+ */
+
+static atomic_t riscv_active_events;
+
+static void riscv_event_destroy(struct perf_event *event)
+{
+ if (atomic_dec_return(&riscv_active_events) == 0)
+ release_pmc_hardware();
+}
+
+static int riscv_event_init(struct perf_event *event)
+{
+ struct perf_event_attr *attr = &event->attr;
+ struct hw_perf_event *hwc = &event->hw;
+ perf_irq_t old_irq_handler = NULL;
+ int code;
+
+ if (atomic_inc_return(&riscv_active_events) == 1)
+ old_irq_handler = reserve_pmc_hardware();
+
+ if (old_irq_handler) {
+ pr_warn("PMC hardware busy (reserved by oprofile)\n");
+ atomic_dec(&riscv_active_events);
+ return -EBUSY;
+ }
+
+ switch (event->attr.type) {
+ case PERF_TYPE_HARDWARE:
+ code = riscv_pmu->map_hw_event(attr->config);
+ break;
+ case PERF_TYPE_HW_CACHE:
+ code = riscv_pmu->map_cache_event(attr->config);
+ break;
+ case PERF_TYPE_RAW:
+ return -EOPNOTSUPP;
+ default:
+ return -ENOENT;
+ }
+
+ event->destroy = riscv_event_destroy;
+ if (code < 0) {
+ event->destroy(event);
+ return code;
+ }
+
+ /*
+ * idx is set to -1 because the index of a general event should not be
+ * decided until binding to some counter in pmu->add().
+ *
+ * But since we don't have such support, later in pmu->add(), we just
+ * use hwc->config as the index instead.
+ */
+ hwc->config = code;
+ hwc->idx = -1;
+
+ return 0;
+}
+
+/*
+ * Initialization
+ */
+
+static struct pmu min_pmu = {
+ .name = "riscv-base",
+ .event_init = riscv_event_init,
+ .add = riscv_pmu_add,
+ .del = riscv_pmu_del,
+ .start = riscv_pmu_start,
+ .stop = riscv_pmu_stop,
+ .read = riscv_pmu_read,
+};
+
+static const struct riscv_pmu riscv_base_pmu = {
+ .pmu = &min_pmu,
+ .max_events = ARRAY_SIZE(riscv_hw_event_map),
+ .map_hw_event = riscv_map_hw_event,
+ .hw_events = riscv_hw_event_map,
+ .map_cache_event = riscv_map_cache_event,
+ .cache_events = &riscv_cache_event_map,
+ .counter_width = 63,
+ .num_counters = RISCV_BASE_COUNTERS + 0,
+};
+
+struct pmu * __weak __init riscv_init_platform_pmu(void)
+{
+ riscv_pmu = &riscv_base_pmu;
+ return riscv_pmu->pmu;
+}
+
+int __init init_hw_perf_events(void)
+{
+ struct pmu *pmu = riscv_init_platform_pmu();
+
+ perf_irq = NULL;
+ perf_pmu_register(pmu, "cpu", PERF_TYPE_RAW);
+ return 0;
+}
+arch_initcall(init_hw_perf_events);
--
2.16.2
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH v3 11/11] dt-bindings: gpio: Add bindings for Cadence I3C gpio expander
From: Geert Uytterhoeven @ 2018-03-26 10:12 UTC (permalink / raw)
To: Boris Brezillon
Cc: Wolfram Sang, Linux I2C, Jonathan Corbet, linux-doc,
Greg Kroah-Hartman, Arnd Bergmann, Przemyslaw Sroka,
Arkadiusz Golec, Alan Douglas, Bartosz Folta, Damian Kos,
Alicja Jurasik-Urbaniak, Cyprian Wronka, Suresh Punnoose,
Rafal Ciepiela, Thomas Petazzoni, Nishanth Menon, Rob Herring,
Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux Kernel Mailing List, Vitor Soares, Linus Walleij, Xiang Lin,
open list:GPIO SUBSYSTEM
In-Reply-To: <20180323110020.19080-12-boris.brezillon@bootlin.com>
Hi Boris,
On Fri, Mar 23, 2018 at 12:00 PM, Boris Brezillon
<boris.brezillon@bootlin.com> wrote:
> Document the Cadence I3C gpio expander bindings.
>
> Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
Thanks for your patch!
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/gpio-cdns-i3c.txt
> +- #interrupt-cells : Should be 2. The first cell is the GPIO number.
> + The second cell bits[3:0] is used to specify trigger type and level flags:
> + 1 = low-to-high edge triggered.
> + 2 = high-to-low edge triggered.
> + 3 = triggered on both edges.
> + 4 = active high level-sensitive.
> + 8 = active low level-sensitive.
These are identical to the values in <dt-bindings/interrupt-controller/irq.h>.
Perhaps you can refer to those definitions?
I don't think we want to see the hardcoded numbers in DTS files.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3 11/11] dt-bindings: gpio: Add bindings for Cadence I3C gpio expander
From: Geert Uytterhoeven @ 2018-03-26 10:17 UTC (permalink / raw)
To: Boris Brezillon
Cc: Wolfram Sang, Linux I2C, Jonathan Corbet, linux-doc,
Greg Kroah-Hartman, Arnd Bergmann, Przemyslaw Sroka,
Arkadiusz Golec, Alan Douglas, Bartosz Folta, Damian Kos,
Alicja Jurasik-Urbaniak, Cyprian Wronka, Suresh Punnoose,
Rafal Ciepiela, Thomas Petazzoni, Nishanth Menon, Rob Herring,
Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux Kernel Mailing List, Vitor Soares, Linus Walleij, Xiang Lin,
open list:GPIO SUBSYSTEM
In-Reply-To: <20180323110020.19080-12-boris.brezillon@bootlin.com>
Hi Boris,
On Fri, Mar 23, 2018 at 12:00 PM, Boris Brezillon
<boris.brezillon@bootlin.com> wrote:
> Document the Cadence I3C gpio expander bindings.
>
> Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
Thanks for your patch!
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/gpio-cdns-i3c.txt
> @@ -0,0 +1,38 @@
> +* Cadence I3C GPIO expander
> +
> +The Cadence I3C GPIO expander provides 8 GPIOs controllable over I3C.
> +This GPIOs can be configured in output or input mode and if they are in input
> +mode they can generate IBIs (In Band Interrupts).
> +
> +Required properties for GPIO node:
> +- reg : 3 cells encoding the I3C static address (none in our case) and the I3C
> + Provisional ID. See Documentation/devicetree/bindings/i3c/i3c.txt for
> + more details.
> + Should be <0x0 0x392 0x0>.
No compatible value?
> +- gpio-controller : Marks the device node as a gpio controller.
> +- #gpio-cells : Should be two. The first cell is the pin number and
> + the second cell is used to specify the gpio polarity:
> + 0 = active high
> + 1 = active low
> +- interrupt-controller: Marks the device node as an interrupt controller.
> +- #interrupt-cells : Should be 2. The first cell is the GPIO number.
> + The second cell bits[3:0] is used to specify trigger type and level flags:
> + 1 = low-to-high edge triggered.
> + 2 = high-to-low edge triggered.
> + 3 = triggered on both edges.
> + 4 = active high level-sensitive.
> + 8 = active low level-sensitive.
> +
> +Example:
> +
> + i3c-master@xxx {
> + ...
> + i3c_gpio_expander: gpio@0,1c9,0 {
gpio@0,392,0?
> + reg = <0 0x392 0x0>;
> + gpio-controller;
> + #gpio-cells = <2>;
> + interrupt-controller;
> + #interrupt-cells = <2>;
> + };
> + ...
> + };
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3 05/11] dt-bindings: i3c: Document core bindings
From: Geert Uytterhoeven @ 2018-03-26 10:22 UTC (permalink / raw)
To: Boris Brezillon
Cc: Wolfram Sang, Linux I2C, Jonathan Corbet, linux-doc,
Greg Kroah-Hartman, Arnd Bergmann, Przemyslaw Sroka,
Arkadiusz Golec, Alan Douglas, Bartosz Folta, Damian Kos,
Alicja Jurasik-Urbaniak, Cyprian Wronka, Suresh Punnoose,
Rafal Ciepiela, Thomas Petazzoni, Nishanth Menon, Rob Herring,
Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux Kernel Mailing List, Vitor Soares, Linus Walleij, Xiang Lin,
open list:GPIO SUBSYSTEM, Boris Brezillon
In-Reply-To: <20180323110020.19080-6-boris.brezillon@bootlin.com>
Hi Boris,
On Fri, Mar 23, 2018 at 12:00 PM, Boris Brezillon
<boris.brezillon@bootlin.com> wrote:
> From: Boris Brezillon <boris.brezillon@free-electrons.com>
>
> A new I3C subsystem has been added and a generic description has been
> created to represent the I3C bus and the devices connected on it.
>
> Document this generic representation.
>
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
Thanks for your patch!
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i3c/i3c.txt
> @@ -0,0 +1,140 @@
> +I3C devices
> +===========
> +
> +All I3C devices are supposed to support DAA (Dynamic Address Assignment), and
> +are thus discoverable. So, by default, I3C devices do not have to be described
> +in the device tree.
But if they're described, they should have a compatible value, no?
> +This being said, one might want to attach extra resources to these devices,
> +and those resources may have to be described in the device tree, which in turn
> +means we have to describe I3C devices.
> +
> +Another use case for describing an I3C device in the device tree is when this
> +I3C device has a static address and we want to assign it a specific dynamic
> +address before the DAA takes place (so that other devices on the bus can't
> +take this dynamic address).
> +
> +The I3C device should be names <device-type>@<static-address>,<i3c-pid>,
named
So the i3c-pid in the unit address is represented as a 64-bit number, not as two
comma-separated 32-bit numbers?
> +Example:
> +
> + i3c-master@d040000 {
> + compatible = "cdns,i3c-master";
> + clocks = <&coreclock>, <&i3csysclock>;
> + clock-names = "pclk", "sysclk";
> + interrupts = <3 0>;
> + reg = <0x0d040000 0x1000>;
> + #address-cells = <3>;
> + #size-cells = <0>;
> +
> + status = "okay";
> + i2c-scl-frequency = <100000>;
> +
> + /* I2C device. */
> + nunchuk: nunchuk@52 {
@52,8000001000000000?
> + compatible = "nintendo,nunchuk";
> + reg = <0x52 0x80000010 0x0>;
> + };
> +
> + /* I3C device with a static address. */
> + thermal_sensor: sensor@68,39200144004 {
No compatible value?
> + reg = <0x68 0x392 0x144004>;
> + assigned-address = <0xa>;
> + };
> +
> + /*
> + * I3C device without a static address but requiring resources
> + * described in the DT.
> + */
> + sensor@0,39200154004 {
No compatible value?
> + reg = <0x0 0x392 0x154004>;
> + clocks = <&clock_provider 0>;
> + };
> + };
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3 05/11] dt-bindings: i3c: Document core bindings
From: Boris Brezillon @ 2018-03-26 11:19 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Wolfram Sang, Linux I2C, Jonathan Corbet, linux-doc,
Greg Kroah-Hartman, Arnd Bergmann, Przemyslaw Sroka,
Arkadiusz Golec, Alan Douglas, Bartosz Folta, Damian Kos,
Alicja Jurasik-Urbaniak, Cyprian Wronka, Suresh Punnoose,
Rafal Ciepiela, Thomas Petazzoni, Nishanth Menon, Rob Herring,
Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux Kernel Mailing List, Vitor Soares, Linus Walleij, Xiang Lin,
open list:GPIO SUBSYSTEM, Boris Brezillon
In-Reply-To: <CAMuHMdV9xW8o6zuOmOxB+=h1XTdi-VFq2mp897SUjZKtRMOWOw@mail.gmail.com>
Hi Geert,
On Mon, 26 Mar 2018 12:22:24 +0200
Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> Hi Boris,
>
> On Fri, Mar 23, 2018 at 12:00 PM, Boris Brezillon
> <boris.brezillon@bootlin.com> wrote:
> > From: Boris Brezillon <boris.brezillon@free-electrons.com>
> >
> > A new I3C subsystem has been added and a generic description has been
> > created to represent the I3C bus and the devices connected on it.
> >
> > Document this generic representation.
> >
> > Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
>
> Thanks for your patch!
>
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/i3c/i3c.txt
> > @@ -0,0 +1,140 @@
>
> > +I3C devices
> > +===========
> > +
> > +All I3C devices are supposed to support DAA (Dynamic Address Assignment), and
> > +are thus discoverable. So, by default, I3C devices do not have to be described
> > +in the device tree.
>
> But if they're described, they should have a compatible value, no?
What's the point of having a compatible here? I mean, I3C devices are
already attached to the relevant drivers using the Provisional ID, why
would we need a compatible if we don't parse it?
>
> > +This being said, one might want to attach extra resources to these devices,
> > +and those resources may have to be described in the device tree, which in turn
> > +means we have to describe I3C devices.
> > +
> > +Another use case for describing an I3C device in the device tree is when this
> > +I3C device has a static address and we want to assign it a specific dynamic
> > +address before the DAA takes place (so that other devices on the bus can't
> > +take this dynamic address).
> > +
> > +The I3C device should be names <device-type>@<static-address>,<i3c-pid>,
>
> named
Oops. Will fix the typo.
>
> So the i3c-pid in the unit address is represented as a 64-bit number, not as two
> comma-separated 32-bit numbers?
Right. I've changed my mind so many times that I ended up mixing the 2
representations I have considered.
Here are the choices we have:
1/ expose the raw ProvisionalID which is a 48-bit integer formed by the
concatenation of the vendor ID, part ID and instance ID:
ProvisionalID = VendorID << 33 | PartID << 16 | InstanceID << 12 |
ExtraInfo
The I3C dev node name should in this case be something like
<device-type>@<static-address>,<i3c-pid>
2/ split the fields we are really interested in in different cells:
2nd cell: vendorID
3rd cell: partID
4th cell: instanceID
In this case, the node name should be
<device-type>@<static-address>,<i3c-vendor>,<i3c-partid>,<i3c-instanceid>
Note that we can still change for the second representation if Rob is
okay.
>
> > +Example:
> > +
> > + i3c-master@d040000 {
> > + compatible = "cdns,i3c-master";
> > + clocks = <&coreclock>, <&i3csysclock>;
> > + clock-names = "pclk", "sysclk";
> > + interrupts = <3 0>;
> > + reg = <0x0d040000 0x1000>;
> > + #address-cells = <3>;
> > + #size-cells = <0>;
> > +
> > + status = "okay";
> > + i2c-scl-frequency = <100000>;
> > +
> > + /* I2C device. */
> > + nunchuk: nunchuk@52 {
>
> @52,8000001000000000?
Well, I2C devs is a special case, and I'm not sure we want to add the
extra LVR information + the IS_I2C_DEV bit in the node name.
>
> > + compatible = "nintendo,nunchuk";
> > + reg = <0x52 0x80000010 0x0>;
One more detail: people are unlikely to define reg using raw values: I
provide macros to do that in patch 6.
> > + };
> > +
> > + /* I3C device with a static address. */
> > + thermal_sensor: sensor@68,39200144004 {
>
> No compatible value?
No, as said above, it's not needed.
>
> > + reg = <0x68 0x392 0x144004>;
> > + assigned-address = <0xa>;
> > + };
> > +
> > + /*
> > + * I3C device without a static address but requiring resources
> > + * described in the DT.
> > + */
> > + sensor@0,39200154004 {
>
> No compatible value?
Ditto.
Thanks for your review.
Boris
--
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3 11/11] dt-bindings: gpio: Add bindings for Cadence I3C gpio expander
From: Boris Brezillon @ 2018-03-26 11:21 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Wolfram Sang, Linux I2C, Jonathan Corbet, linux-doc,
Greg Kroah-Hartman, Arnd Bergmann, Przemyslaw Sroka,
Arkadiusz Golec, Alan Douglas, Bartosz Folta, Damian Kos,
Alicja Jurasik-Urbaniak, Cyprian Wronka, Suresh Punnoose,
Rafal Ciepiela, Thomas Petazzoni, Nishanth Menon, Rob Herring,
Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux Kernel Mailing List, Vitor Soares, Linus Walleij, Xiang Lin,
open list:GPIO SUBSYSTEM
In-Reply-To: <CAMuHMdUH3iB6ngF-0Zkij-oNN-Oms=EwXgoceLp06xbdcq1S=w@mail.gmail.com>
Hi Geert,
On Mon, 26 Mar 2018 12:17:26 +0200
Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> Hi Boris,
>
> On Fri, Mar 23, 2018 at 12:00 PM, Boris Brezillon
> <boris.brezillon@bootlin.com> wrote:
> > Document the Cadence I3C gpio expander bindings.
> >
> > Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
>
> Thanks for your patch!
>
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/gpio/gpio-cdns-i3c.txt
> > @@ -0,0 +1,38 @@
> > +* Cadence I3C GPIO expander
> > +
> > +The Cadence I3C GPIO expander provides 8 GPIOs controllable over I3C.
> > +This GPIOs can be configured in output or input mode and if they are in input
> > +mode they can generate IBIs (In Band Interrupts).
> > +
> > +Required properties for GPIO node:
> > +- reg : 3 cells encoding the I3C static address (none in our case) and the I3C
> > + Provisional ID. See Documentation/devicetree/bindings/i3c/i3c.txt for
> > + more details.
> > + Should be <0x0 0x392 0x0>.
>
> No compatible value?
See my other reply.
>
> > +- gpio-controller : Marks the device node as a gpio controller.
> > +- #gpio-cells : Should be two. The first cell is the pin number and
> > + the second cell is used to specify the gpio polarity:
> > + 0 = active high
> > + 1 = active low
> > +- interrupt-controller: Marks the device node as an interrupt controller.
> > +- #interrupt-cells : Should be 2. The first cell is the GPIO number.
> > + The second cell bits[3:0] is used to specify trigger type and level flags:
> > + 1 = low-to-high edge triggered.
> > + 2 = high-to-low edge triggered.
> > + 3 = triggered on both edges.
> > + 4 = active high level-sensitive.
> > + 8 = active low level-sensitive.
> > +
> > +Example:
> > +
> > + i3c-master@xxx {
> > + ...
> > + i3c_gpio_expander: gpio@0,1c9,0 {
>
> gpio@0,392,0?
Actually, if I follow the DT bindings, it should be gpio@0,39200000000
Thanks,
Boris
--
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3 11/11] dt-bindings: gpio: Add bindings for Cadence I3C gpio expander
From: Boris Brezillon @ 2018-03-26 11:25 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Wolfram Sang, Linux I2C, Jonathan Corbet, linux-doc,
Greg Kroah-Hartman, Arnd Bergmann, Przemyslaw Sroka,
Arkadiusz Golec, Alan Douglas, Bartosz Folta, Damian Kos,
Alicja Jurasik-Urbaniak, Cyprian Wronka, Suresh Punnoose,
Rafal Ciepiela, Thomas Petazzoni, Nishanth Menon, Rob Herring,
Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux Kernel Mailing List, Vitor Soares, Linus Walleij, Xiang Lin,
open list:GPIO SUBSYSTEM
In-Reply-To: <CAMuHMdX3TLmaN14gM4fTgzXwy1dgsMxCFBs_3wCFWM8rX=+8vA@mail.gmail.com>
On Mon, 26 Mar 2018 12:12:54 +0200
Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> Hi Boris,
>
> On Fri, Mar 23, 2018 at 12:00 PM, Boris Brezillon
> <boris.brezillon@bootlin.com> wrote:
> > Document the Cadence I3C gpio expander bindings.
> >
> > Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
>
> Thanks for your patch!
>
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/gpio/gpio-cdns-i3c.txt
>
> > +- #interrupt-cells : Should be 2. The first cell is the GPIO number.
> > + The second cell bits[3:0] is used to specify trigger type and level flags:
> > + 1 = low-to-high edge triggered.
> > + 2 = high-to-low edge triggered.
> > + 3 = triggered on both edges.
> > + 4 = active high level-sensitive.
> > + 8 = active low level-sensitive.
>
> These are identical to the values in <dt-bindings/interrupt-controller/irq.h>.
> Perhaps you can refer to those definitions?
Well, I'm not sure this is allowed since DT bindings docs are
supposed to be OS-agnostic and macros defined in
<dt-bindings/interrupt-controller/irq.h> are, AFAIK, only available to
Linux.
Note that I copied these definitions from another binding ;-).
Rob, any opinion?
--
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3 11/11] dt-bindings: gpio: Add bindings for Cadence I3C gpio expander
From: Geert Uytterhoeven @ 2018-03-26 11:35 UTC (permalink / raw)
To: Boris Brezillon
Cc: Wolfram Sang, Linux I2C, Jonathan Corbet, linux-doc,
Greg Kroah-Hartman, Arnd Bergmann, Przemyslaw Sroka,
Arkadiusz Golec, Alan Douglas, Bartosz Folta, Damian Kos,
Alicja Jurasik-Urbaniak, Cyprian Wronka, Suresh Punnoose,
Rafal Ciepiela, Thomas Petazzoni, Nishanth Menon, Rob Herring,
Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux Kernel Mailing List, Vitor Soares, Linus Walleij, Xiang Lin,
open list:GPIO SUBSYSTEM
In-Reply-To: <20180326132526.1634946e@bbrezillon>
Hi Boris,
On Mon, Mar 26, 2018 at 1:25 PM, Boris Brezillon
<boris.brezillon@bootlin.com> wrote:
> On Mon, 26 Mar 2018 12:12:54 +0200
> Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>> On Fri, Mar 23, 2018 at 12:00 PM, Boris Brezillon
>> <boris.brezillon@bootlin.com> wrote:
>> > Document the Cadence I3C gpio expander bindings.
>> >
>> > Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
>>
>> Thanks for your patch!
>>
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/gpio/gpio-cdns-i3c.txt
>>
>> > +- #interrupt-cells : Should be 2. The first cell is the GPIO number.
>> > + The second cell bits[3:0] is used to specify trigger type and level flags:
>> > + 1 = low-to-high edge triggered.
>> > + 2 = high-to-low edge triggered.
>> > + 3 = triggered on both edges.
>> > + 4 = active high level-sensitive.
>> > + 8 = active low level-sensitive.
>>
>> These are identical to the values in <dt-bindings/interrupt-controller/irq.h>.
>> Perhaps you can refer to those definitions?
>
> Well, I'm not sure this is allowed since DT bindings docs are
> supposed to be OS-agnostic and macros defined in
> <dt-bindings/interrupt-controller/irq.h> are, AFAIK, only available to
> Linux.
If they're under include/dt-bindings/, they're part of the DT bindings.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v6 2/2] cpuset: Add cpuset.sched_load_balance to v2
From: Juri Lelli @ 2018-03-26 12:47 UTC (permalink / raw)
To: Waiman Long
Cc: Tejun Heo, Li Zefan, Johannes Weiner, Peter Zijlstra, Ingo Molnar,
cgroups, linux-kernel, linux-doc, kernel-team, pjt, luto, efault,
torvalds, Roman Gushchin
In-Reply-To: <a8860a42-31b5-4620-7b23-b7515ee11d7d@redhat.com>
On 23/03/18 14:44, Waiman Long wrote:
> On 03/23/2018 03:59 AM, Juri Lelli wrote:
[...]
> > OK, thanks for confirming. Can you tell again however why do you think
> > we need to remove sched_load_balance from root level? Won't we end up
> > having tasks put on isolated sets?
>
> The root cgroup is special that it owns all the resources in the system.
> We generally don't want restriction be put on the root cgroup. A child
> cgroup has to be created to have constraints put on it. In fact, most of
> the controller files don't show up in the v2 cgroup root at all.
>
> An isolated cgroup has to be put under root, e.g.
>
> Root
> / \
> isolated balanced
>
> >
> > Also, I guess children groups with more than one CPU will need to be
> > able to load balance across their CPUs, no matter what their parent
> > group does?
>
> The purpose of an isolated cpuset is to have a dedicated set of CPUs to
> be used by a certain application that makes its own scheduling decision
> by placing tasks explicitly on specific CPUs. It just doesn't make sense
> to have a CPU in an isolated cpuset to participated in load balancing in
> another cpuset. If one want load balancing in a child cpuset, the parent
> cpuset should have load balancing turned on as well.
Isolated with CPUs overlapping some other cpuset makes little sense, I
agree. What I have in mind however is an isolated set of CPUs that don't
overlap with any other cpuset (as your balanced set above). In this case
I think it makes sense to let the sys admin decide if "automatic" load
balancing has to be performed (by the scheduler) or no load balacing at
all has to take place?
Further extending your example:
Root [0-3]
/ \
group1 [0-1] group2[2-3]
Why should we prevent load balancing to be disabled at root level (so
that for example tasks still residing in root group are not freely
migrated around, potentially disturbing both sub-groups)?
Then one can decide that group1 is a "userspace managed" group (no load
balancing takes place) and group2 is balanced by the scheduler.
And this is not DEADLINE specific, IMHO.
> As I look into the code, it seems like root domain is probably somewhat
> associated with cpu_exclusive only. Whether sched_load_balance is set
> doesn't really matter. I will need to look further on the conditions
> where a new root domain is created.
I checked again myself (sched domains code is always a maze :) and I
believe that sched_load_balance flag indeed controls domains (sched and
root) creation and configuration . Changing the flag triggers potential
rebuild and separed sched/root domains are generated if subgroups have
non overlapping cpumasks. cpu_exclusive only enforces this latter
condition.
Best,
- Juri
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] Input: alps - Update documentation for trackstick v3 format
From: Jonathan Corbet @ 2018-03-26 14:41 UTC (permalink / raw)
To: Pali Rohár
Cc: Dmitry Torokhov, Masaki Ota, linux-input, linux-doc, linux-kernel
In-Reply-To: <20180321200336.32035-1-pali.rohar@gmail.com>
On Wed, 21 Mar 2018 21:03:36 +0100
Pali Rohár <pali.rohar@gmail.com> wrote:
> Bits for M, R and L buttons are already processed in alps. Other newly
> documented bits not yet.
>
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> ---
> This is based on information which Masaki Ota provided to us.
In the absence of screams of agony I'll assume that the change is
correct. Applied to the docs tree, thanks.
jon
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] Documentation: admin-guide: add kvmconfig, xenconfig and tinyconfig commands
From: Jonathan Corbet @ 2018-03-26 14:41 UTC (permalink / raw)
To: Martin Kepplinger; +Cc: linux-doc, linux-kernel
In-Reply-To: <20180322120656.13644-1-martink@posteo.de>
On Thu, 22 Mar 2018 13:06:56 +0100
Martin Kepplinger <martink@posteo.de> wrote:
> Add kvmconfig, xenconfig and tinyconfig to the list of alternative
> configuration commands. Descriptions are directly taken from the Makefile.
Applied to the docs tree, thanks.
jon
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] Documentation: magic-numbers: Fix typo
From: Jonathan Corbet @ 2018-03-26 14:42 UTC (permalink / raw)
To: Martin Kepplinger; +Cc: linux-doc, linux-kernel
In-Reply-To: <20180323073231.5826-1-martink@posteo.de>
On Fri, 23 Mar 2018 08:32:31 +0100
Martin Kepplinger <martink@posteo.de> wrote:
> This fixes a little then / them confusion.
>
> Signed-off-by: Martin Kepplinger <martink@posteo.de>
> ---
> Documentation/process/magic-number.rst | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/process/magic-number.rst b/Documentation/process/magic-number.rst
> index c74199f60c6c..00cecf1fcba9 100644
> --- a/Documentation/process/magic-number.rst
> +++ b/Documentation/process/magic-number.rst
> @@ -14,7 +14,7 @@ passing pointers to structures via a void * pointer. The tty code,
> for example, does this frequently to pass driver-specific and line
> discipline-specific structures back and forth.
>
> -The way to use magic numbers is to declare then at the beginning of
> +The way to use magic numbers is to declare them at the beginning of
> the structure, like so::
>
> struct tty_ldisc {
Applied, thanks.
jon
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH v3] Documentation/CodingStyle: Add an example for braces
From: Gary R Hook @ 2018-03-26 16:24 UTC (permalink / raw)
To: linux-doc; +Cc: linux-kernel, corbet
Add another example of required braces when using a compound statement in
a loop.
Signed-off-by: Gary R Hook <gary.hook@amd.com>
---
Changes since v2:
- Modified the example code fragment
Changes since v1:
- Move the new example up, and make it more generic
Documentation/process/coding-style.rst | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/Documentation/process/coding-style.rst b/Documentation/process/coding-style.rst
index a20b44a40ec4..24903244c8be 100644
--- a/Documentation/process/coding-style.rst
+++ b/Documentation/process/coding-style.rst
@@ -188,6 +188,15 @@ and
else
do_that();
+Do use braces when a body is more complex than a single simple statement:
+
+.. code-block:: c
+
+ if (condition) {
+ while (some_loop_condition)
+ do_something();
+ }
+
This does not apply if only one branch of a conditional statement is a single
statement; in the latter case use braces in both branches:
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH v2] Documentation/CodingStyle: Add an example for braces
From: Gary R Hook @ 2018-03-26 16:28 UTC (permalink / raw)
To: Jani Nikula, Jonathan Corbet; +Cc: linux-doc, linux-kernel
In-Reply-To: <87woy4iide.fsf@intel.com>
On 03/22/2018 04:12 AM, Jani Nikula wrote:
> On Wed, 21 Mar 2018, Jonathan Corbet <corbet@lwn.net> wrote:
>> To head that off, I think I'll apply your first version instead, sorry
>> Jani.
>
> No worries.
>
Submitting a v3 because the example could better illuminate the options
by using loop construct inside of an if, addressing Jani's point but
without opening the door to later criticism.
I also like the verbage in v2/3 better, but I'll let Jonathan make the call.
BTW which tree should these be developed against? I used torvalds, but
I'm not entirely sure that was the proper one?
Gary
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox