Re: [PATCH 000/141] Fix fall-through warnings for Clang
Hi James. > > > If none of the 140 patches here fix a real bug, and there is no > > > change to machine code then it sounds to me like a W=2 kind of a > > > warning. > > > > FWIW, this series has found at least one bug so far: > > https://lore.kernel.org/lkml/CAFCwf11izHF=g1mGry1fE5kvFFFrxzhPSM6qKAO8gxSp=kr...@mail.gmail.com/ > > > Well, it's a problem in an error leg, sure, but it's not a really > compelling reason for a 141 patch series, is it? All that fixing this > error will do is get the driver to print "oh dear there's a problem" > under four more conditions than it previously did. You are asking the wrong question here. Yuo should ask how many hours could have been saved by all the bugs people have been fighting with and then fixed *before* the code hit the kernel at all. My personal experience is that I, more than once, have had errors related to a missing break in my code. So this warnings is IMO a win. And if we are only ~100 patches to have it globally enabled then it is a no-brainer in my book. Sam
Re: [PATCH 00/49] DRM driver for Hikey 970
Hi Mauro.
Laurent and I discussed this driver a little on irc.
Some highlights:
This parts could use register names:
+ writel(0x2, noc_dss_base + 0xc);
+ writel(0x2, noc_dss_base + 0x8c);
+ writel(0x2, noc_dss_base + 0x10c);
+ writel(0x2, noc_dss_base + 0x18c);
The two nodes in the DT for DPE and DSI uses overlapping range for reg
entries. It looks like a syscon node or some iommu thing is needed to do
this properly.
The chain will lok like this:
DPE -> DSI -> video mux -> {adv7533, panel}
But drm_bridge has not yet support for such non-linear setup.
The recommendation is to focus on the HDMI prat. Then we can later
come up with support for a video mux.
The video mux should have a dedicated node with one input node and two
output nodes. Which is also where the gpio should be.
The DSI node references two DPHY instances - should it be PHY driver(s)?
Does the DSI part contain one or two instances. Clocks looks duplicated.
Does the DPE and DSI share a lot of register blocks - or does it just
look like this from a first point of view?
You can read though the logs here:
https://people.freedesktop.org/~cbrill/dri-log/index.php
Could you please try to get back on some of the points above so we can
help you move forward in the right direction.
Thanks,
Sam
Re: [PATCH 00/49] DRM driver for Hikey 970
Hi Mauro
> Before posting the big patch series again, let me send the new
> version folded into a single patch.
>
> If you'd like to see the entire thing, I'm placing it here:
>
>
> https://gitlab.freedesktop.org/mchehab_kernel/hikey-970/-/commits/hikey970_v2/
Review 3/3
For next submission then to ease review it would be nice that the patch
is spilt up a little. Maybe something like:
- HW specific stuff in two patches (header fiels with register + setup
code)
- Display driver files
- DSI driver files
- build stuff (Makefile, Kconfig fragments)
So all splits on file level - which should be easy to do.
This will make it easier for more people to give feedback. It is a bit
much to walk through 4k loc or what the full size is.
And then we can also provide a-b or r-b for fragments so the reviewed
parts can be ignored for later reviews.
For the DSI parts I may be hit by the old "when you have a hammer then
everything looks like a nail syndrome".
In my case the hammer is the bridge interface.
Suggestions:
- Move encoder to display driver
- Move connector creation to display driver (using
drm_bridge_connector_init())
- Use drm_bridge interface for the DSI driver parts
- Maybe use the HDMI bridge driver as a chained driver - I did not look
to close on this
- Use the standard bridge interface to find the bridge and drop use of
the component framework
I hope that some other drm people chime in here.
Either to tell this is non-sense or confirm this is the way to go.
The above does not give any hint how to use the gpio_mux to shift
between the panel and HDMI. This logic needs to be in the display driver
as the bridge driver will only be used for HDMI - as I understand the
code. And I do not know how to do it.
Some details in the following that are not related to the above.
Sam
> diff --git a/drivers/staging/hikey9xx/gpu/kirin9xx_dw_drm_dsi.c
> b/drivers/staging/hikey9xx/gpu/kirin9xx_dw_drm_dsi.c
> new file mode 100644
> index ..a2eed961b7d5
> --- /dev/null
> +++ b/drivers/staging/hikey9xx/gpu/kirin9xx_dw_drm_dsi.c
> @@ -0,0 +1,2126 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * DesignWare MIPI DSI Host Controller v1.02 driver
> + *
> + * Copyright (c) 2016 Linaro Limited.
> + * Copyright (c) 2014-2016 Hisilicon Limited.
> + * Copyright (c) 2014-2020, Huawei Technologies Co., Ltd
> + *
> + * Author:
> + *
> + *
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
Sort
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
Sort
> +
> +#include "kirin9xx_dw_dsi_reg.h"
> +#include "kirin9xx_dpe.h"
> +#include "kirin9xx_drm_drv.h"
> +#include "kirin9xx_drm_dpe_utils.h"
Sort
> +
> +#define PHY_REF_CLK_RATE 1920
> +#define PHY_REF_CLK_PERIOD_PS(10 / (PHY_REF_CLK_RATE / 1000))
> +
> +#define encoder_to_dsi(encoder) \
> + container_of(encoder, struct dw_dsi, encoder)
> +#define host_to_dsi(host) \
> + container_of(host, struct dw_dsi, host)
> +#define connector_to_dsi(connector) \
> + container_of(connector, struct dw_dsi, connector)
Move the upcast next to the struct definition.
> +#define DSS_REDUCE(x)((x) > 0 ? ((x) - 1) : (x))
> +
> +enum dsi_output_client {
> + OUT_HDMI = 0,
> + OUT_PANEL,
> + OUT_MAX
> +};
> +
> +struct mipi_phy_params {
I did not check all fiels but I managed to find at least one unused
field. Cheack and delete what is unused.
> + u64 lane_byte_clk;
> + u32 clk_division;
> +
> + u32 clk_lane_lp2hs_time;
> + u32 clk_lane_hs2lp_time;
> + u32 data_lane_lp2hs_time;
> + u32 data_lane_hs2lp_time;
> + u32 clk2data_delay;
> + u32 data2clk_delay;
> +
> + u32 clk_pre_delay;
> + u32 clk_post_delay;
> + u32 clk_t_lpx;
> + u32 clk_t_hs_prepare;
> + u32 clk_t_hs_zero;
> + u32 clk_t_hs_trial;
> + u32 clk_t_wakeup;
> + u32 data_pre_delay;
> + u32 data_post_delay;
> + u32 data_t_lpx;
> + u32 data_t_hs_prepare;
> + u32 data_t_hs_zero;
> + u32 data_t_hs_trial;
> + u32 data_t_ta_go;
> + u32 data_t_ta_get;
> + u32 data_t_wakeup;
> +
> + u32 phy_stop_wait_time;
> +
> + u32 rg_vrefsel_vcm;
> + u32 rg_hstx_ckg_sel;
> + u32 rg_pll_fbd_div5f;
> + u32 rg_pll_fbd_div1f;
> + u32 rg_pll_fbd_2p;
> + u32 rg_pll_enbwt;
> + u32 rg_pll_fbd_p;
> + u32 rg_pll_fbd_s;
> + u32 rg_pll_pre_div1p;
> + u32 rg_pll_pre_p;
> + u32 rg_pll_vco_750m;
> + u32 rg_pll_lpf_rs;
> + u32 rg_pll_lpf_cs;
> + u32 rg_pll_enswc;
> + u32 rg_pll_chp;
> +
Some indent gone wrong here?
> + u32 pll_register_override; /* 0x1E[0] */
> + u32 pll_power_down; /* 0x1E[1] */
> + u32 rg_band_sel;/* 0x1E[2] */
> + u32 rg_phase_gen_en;/* 0x1E[3] */
> + u32 reload_sel; /
Re: [PATCH 00/49] DRM driver for Hikey 970
Hi Mauro
> Before posting the big patch series again, let me send the new
> version folded into a single patch.
Review 2/N
The way output_poll_changed is used to set gpio_mux to select between
the panel and the HDMI looks strange. But I do not know if there is a
more correct way to do it. Other DRM people would need to help
here if there is a better way to do it.
I looked briefly af suspend/resume.
I think this would benefit from using drm_mode_config_helper_suspend()
and drm_mode_config_helper_resume() but I did not finalize the anlyzis.
Other than this only some small details in the following.
Sam
> kirin9xx_drm_drv.c b/drivers/staging/hikey9xx/gpu/kirin9xx_drm_drv.c
> new file mode 100644
> index ..61b65f8b1ace
> --- /dev/null
> +++ b/drivers/staging/hikey9xx/gpu/kirin9xx_drm_drv.c
> @@ -0,0 +1,277 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Hisilicon Kirin SoCs drm master driver
> + *
> + * Copyright (c) 2016 Linaro Limited.
> + * Copyright (c) 2014-2016 Hisilicon Limited.
> + * Copyright (c) 2014-2020, Huawei Technologies Co., Ltd
> + * Author:
> + *
> + *
> + */
> +
> +#include
> +#include
> +#include
Sort includes
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
Sort includes
> +
> +#include "kirin9xx_dpe.h"
> +#include "kirin9xx_drm_drv.h"
> +
> +static int kirin_drm_kms_cleanup(struct drm_device *dev)
> +{
> + struct kirin_drm_private *priv = to_drm_private(dev);
> +
> + if (priv->fbdev)
> + priv->fbdev = NULL;
> +
> + drm_kms_helper_poll_fini(dev);
> + kirin9xx_dss_drm_cleanup(dev);
> +
> + return 0;
> +}
> +
> +static void kirin_fbdev_output_poll_changed(struct drm_device *dev)
> +{
> + struct kirin_drm_private *priv = to_drm_private(dev);
> +
> + dsi_set_output_client(dev);
> +
> + drm_fb_helper_hotplug_event(priv->fbdev);
> +}
> +
> +static const struct drm_mode_config_funcs kirin_drm_mode_config_funcs = {
> + .fb_create = drm_gem_fb_create,
> + .output_poll_changed = kirin_fbdev_output_poll_changed,
> + .atomic_check = drm_atomic_helper_check,
> + .atomic_commit = drm_atomic_helper_commit,
> +};
> +
> +static int kirin_drm_kms_init(struct drm_device *dev)
> +{
> + long kirin_type;
> + int ret;
> +
> + dev_set_drvdata(dev->dev, dev);
> +
> + ret = drmm_mode_config_init(dev);
> + if (ret)
> + return ret;
> +
> + dev->mode_config.min_width = 0;
> + dev->mode_config.min_height = 0;
> + dev->mode_config.max_width = 2048;
> + dev->mode_config.max_height = 2048;
> + dev->mode_config.preferred_depth = 32;
> +
> + dev->mode_config.funcs = &kirin_drm_mode_config_funcs;
> +
> + /* display controller init */
> + kirin_type = (long)of_device_get_match_data(dev->dev);
> + if (WARN_ON(!kirin_type))
> + return -ENODEV;
> +
> + ret = dss_drm_init(dev, kirin_type);
> + if (ret)
> + return ret;
> +
> + /* bind and init sub drivers */
> + ret = component_bind_all(dev->dev, dev);
> + if (ret) {
> + drm_err(dev, "failed to bind all component.\n");
> + return ret;
> + }
> +
> + /* vblank init */
> + ret = drm_vblank_init(dev, dev->mode_config.num_crtc);
> + if (ret) {
> + drm_err(dev, "failed to initialize vblank.\n");
> + return ret;
> + }
> + /* with irq_enabled = true, we can use the vblank feature. */
> + dev->irq_enabled = true;
> +
> + /* reset all the states of crtc/plane/encoder/connector */
> + drm_mode_config_reset(dev);
> +
> + /* init kms poll for handling hpd */
> + drm_kms_helper_poll_init(dev);
> +
> + return 0;
> +}
> +
> +DEFINE_DRM_GEM_CMA_FOPS(kirin_drm_fops);
Move it to right above kirin_drm_driver where it is used
> +
> +static int kirin_drm_connectors_register(struct drm_device *dev)
> +{
> + struct drm_connector_list_iter conn_iter;
> + struct drm_connector *failed_connector;
> + struct drm_connector *connector;
> + int ret;
> +
> + mutex_lock(&dev->mode_config.mutex);
> + drm_connector_list_iter_begin(dev, &conn_iter);
> + drm_for_each_connector_iter(connector, &conn_iter) {
> + ret = drm_connector_register(connector);
> + if (ret) {
> + failed_connector = connector;
> + goto err;
> + }
> + }
> + mutex_unlock(&dev->mode_config.mutex);
> +
> + return 0;
> +
> +err:
> + drm_connector_list_iter_begin(dev, &conn_iter);
> + drm_for_each_connector_iter(connector, &conn_iter) {
> + if (failed_connector == connector)
> + break;
> + drm_connector_unregister(connector);
> + }
> + mutex_unlock(&dev->mode_config.mutex);
> +
> + return ret;
> +}
> +
> +static struct drm_driver kirin_drm_driver = {
> + .driver_featur
Re: [PATCH 00/49] DRM driver for Hikey 970
Hi Mauro > kirin9xx_fb_panel.h b/drivers/staging/hikey9xx/gpu/kirin9xx_fb_panel.h > new file mode 100644 > index ..a69c20470f1d > --- /dev/null > +++ b/drivers/staging/hikey9xx/gpu/kirin9xx_fb_panel.h This file is not referenced and should be deleted. Sam
Re: [PATCH 00/49] DRM driver for Hikey 970
Hi Mauro.
> Before posting the big patch series again, let me send the new
> version folded into a single patch.
Review 1/N
Lots of small details I missed last time.
A good thing is that there is an opportunity to delete som more code.
Sam
> diff --git a/drivers/staging/hikey9xx/gpu/Kconfig
> b/drivers/staging/hikey9xx/gpu/Kconfig
> new file mode 100644
> index ..8578ca953785
> --- /dev/null
> +++ b/drivers/staging/hikey9xx/gpu/Kconfig
> @@ -0,0 +1,10 @@
> +config DRM_HISI_KIRIN9XX
> + tristate "DRM Support for Hisilicon Kirin9xx series SoCs Platform"
> + depends on DRM && OF && ARM64
> + select DRM_KMS_HELPER
> + select DRM_GEM_CMA_HELPER
> + select DRM_KMS_CMA_HELPER
> + select DRM_MIPI_DSI
> + help
> + Choose this option if you have a HiSilicon Kirin960 or Kirin970.
> + If M is selected the module will be called kirin9xx-drm.
> diff --git a/drivers/staging/hikey9xx/gpu/Makefile
> b/drivers/staging/hikey9xx/gpu/Makefile
> new file mode 100644
> index ..5f7974a95367
> --- /dev/null
> +++ b/drivers/staging/hikey9xx/gpu/Makefile
> @@ -0,0 +1,8 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +kirin9xx-drm-y := kirin9xx_drm_drv.o \
> + kirin9xx_drm_dss.o \
> + kirin9xx_drm_dpe_utils.o \
> + kirin970_defs.o kirin960_defs.o \
> + kirin9xx_drm_overlay_utils.o
> +
> +obj-$(CONFIG_DRM_HISI_KIRIN9XX) += kirin9xx-drm.o kirin9xx_dw_drm_dsi.o
General comment which is true for many many Makefile's
I have never understood the love of '\'.
Something like this works equally well:
kirin9xx-drm-y := kirin9xx_drm_drv.o kirin9xx_drm_dss.o
kirin9xx-drm-y += kirin9xx_drm_dpe_utils.o kirin970_defs.o
kirin9xx-drm-y += kirin960_defs.o kirin9xx_drm_overlay_utils.o
obj-$(CONFIG_DRM_HISI_KIRIN9XX) += kirin9xx-drm.o kirin9xx_dw_drm_dsi.o
> diff --git a/drivers/staging/hikey9xx/gpu/kirin960_defs.c
> b/drivers/staging/hikey9xx/gpu/kirin960_defs.c
> new file mode 100644
> index ..c5e1ec03c818
> --- /dev/null
> +++ b/drivers/staging/hikey9xx/gpu/kirin960_defs.c
> @@ -0,0 +1,346 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2008-2011, Hisilicon Tech. Co., Ltd. All rights reserved.
> + * Copyright (c) 2008-2020, Huawei Technologies Co., Ltd
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "kirin9xx_drm_dpe_utils.h"
> +#include "kirin9xx_drm_drv.h"
> +#include "kirin960_dpe_reg.h"
All includes blocks should be sorted.
The list of include files looks far too large for this simple file.
Reduce to the relevant include files.
> +
> +static const u32
> kirin960_g_dss_module_base[DSS_CHN_MAX_DEFINE][MODULE_CHN_MAX] = {
> + {
> + /* D0 */
> + MIF_CH0_OFFSET,
> + AIF0_CH0_OFFSET,
> + AIF1_CH0_OFFSET,
> + MCTL_CTL_MUTEX_RCH0,
> + DSS_MCTRL_SYS_OFFSET + MCTL_RCH0_FLUSH_EN,
> + DSS_MCTRL_SYS_OFFSET + MCTL_RCH0_OV_OEN,
> + DSS_MCTRL_SYS_OFFSET + MCTL_RCH0_STARTY,
> + DSS_MCTRL_SYS_OFFSET + MCTL_MOD0_DBG,
> + DSS_RCH_D0_DMA_OFFSET,
> + DSS_RCH_D0_DFC_OFFSET,
> + 0,
> + 0,
> + 0,
> + 0,
> + 0,
> + 0,
> + DSS_RCH_D0_CSC_OFFSET,
> + }, {
...
> + },
> +};
> +
> +static const u32
> kirin960_g_dss_module_ovl_base[DSS_MCTL_IDX_MAX][MODULE_OVL_MAX] = {
> + {
> + DSS_OVL0_OFFSET,
> + DSS_MCTRL_CTL0_OFFSET
> + }, {
> + DSS_OVL1_OFFSET,
> + DSS_MCTRL_CTL1_OFFSET
> + }, {
> + DSS_OVL2_OFFSET,
> + DSS_MCTRL_CTL2_OFFSET,
> + }, {
> + DSS_OVL3_OFFSET,
> + DSS_MCTRL_CTL3_OFFSET,
> + }, {
> + 0,
> + DSS_MCTRL_CTL4_OFFSET,
> + }, {
> + 0,
> + DSS_MCTRL_CTL5_OFFSET,
> + }
> +};
> +
> +/* SCF_LUT_CHN coef_idx */
> +static const int kirin960_g_scf_lut_chn_coef_idx[DSS_CHN_MAX_DEFINE] = {
> + -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1
> +};
> +
> +static const u32
> kirin960_g_dss_module_cap[DSS_CHN_MAX_DEFINE][MODULE_CAP_MAX] = {
> + /* D2 */
> + {0, 0, 1, 0, 0, 0, 1, 0, 0, 0, 1},
> + /* D3 */
> + {0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 1},
> + /* V0 */
> + {0, 1, 1, 0, 1, 1, 1, 0, 0, 1, 1},
> + /* G0 */
> + {0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0},
> + /* V1 */
> + {0, 1, 1, 1, 0, 1, 1, 0, 1, 1, 1},
> + /* G1 */
> + {0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0},
> + /* D0 */
> + {0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 1},
> + /* D1 */
> + {0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 1},
> +
> + /* W0 */
> + {1, 0, 1, 0, 0, 0, 0, 1, 0, 1, 1},
> + /* W1 */
> + {1, 0, 1, 0, 0, 0, 0, 1, 0, 1, 1},
> +
> + /* V2 */
> + {0, 1, 1, 1, 0, 1, 1, 0, 1, 1, 1},
> + /* W2 */
> +
Re: [PATCH 00/49] DRM driver for Hikey 970
Hi Mauro. On Fri, Aug 21, 2020 at 04:41:58PM +0200, Mauro Carvalho Chehab wrote: > Another quick question: > > Em Wed, 19 Aug 2020 19:35:58 +0200 > Sam Ravnborg escreveu: > > > > +#define DSS_REDUCE(x)((x) > 0 ? ((x) - 1) : (x)) > > Use generic macros for this? > > Do you know a generic macro similar to this? Or do you mean adding > it to include/kernel.h? It looked like something there should be a macro for. But I do not know one. And no, do not try to go the kernel.h route on this. At least not until you see more than one user. Sam
Re: [PATCH 00/49] DRM driver for Hikey 970
Hi Mauro.
Thanks for the detailed feedabck.
Two comments in the following.
Sam
>
> > > + ctx->dss_pri_clk = devm_clk_get(dev, "clk_edc0");
> > > + if (!ctx->dss_pri_clk) {
> > > + DRM_ERROR("failed to parse dss_pri_clk\n");
> > > + return -ENODEV;
> > > + }
> ...
>
> > I had expected some of these could fail with a PROBE_DEFER.
> > Consider to use the newly introduced dev_probe_err()
>
> Yeah, getting clock lines can fail. I was unable to find dev_probe_err(),
> at least on Kernel 5.9-rc1. I saw this comment:
>
> https://lkml.org/lkml/2020/3/6/356
>
> It sounds it didn't reach upstream. Anyway, I add error handling for the
> the clk_get calls:
>
> ctx->dss_pri_clk = devm_clk_get(dev, "clk_edc0");
> ret = PTR_ERR_OR_ZERO(ctx->dss_pri_clk);
> if (ret == -EPROBE_DEFER) {
> return ret;
> } else if (ret) {
> DRM_ERROR("failed to parse dss_pri_clk: %d\n", ret);
> return ret;
> }
>
> This should be able to detect deferred probe, plus to warn
> about other errors.
I got the name wrong. It is named dev_err_probe(), and was introduced in -rc1.
> > Can the panel stuff be moved out and utilise drm_panel?
>
> I saw the code at drm_panel. The real issue here is that I can't
> test anything related to panel support, as I lack any hardware
> for testing. So, there's a high chance I may end breaking
> something while trying to do that.
I will try to take a look again when you post next revision.
Maybe we should update it and risk that is not works, so whenever
someone try to fix it they do so on top of an up-to-date implmentation.
Lets se and decide later.
Sam
Re: [PATCH 00/49] DRM driver for Hikey 970
Hi Mauro.
Quick feedback below.
Sam
On Thu, Aug 20, 2020 at 05:13:22PM +0200, Mauro Carvalho Chehab wrote:
> Em Thu, 20 Aug 2020 16:48:08 +0200
> Sam Ravnborg escreveu:
>
> > Hi Mauro.
> >
> > On Thu, Aug 20, 2020 at 04:06:49PM +0200, Mauro Carvalho Chehab wrote:
> > > Em Wed, 19 Aug 2020 19:35:58 +0200
> > > Sam Ravnborg escreveu:
> > >
> > > I'm already handling the other comments from your review (I'll send a
> > > more complete comment about them after finishing),
> > If you get back only on things you do not understand or do not agree on
> > that would be fine. The rest should be visible in the changelog on the
> > updated patch - no need to do extra work here.
> >
> > > but I have a doubt what you meant about this:
> > >
> > > > +static int kirin_drm_bind(struct device *dev)
> > > > > +{
> > > > > + struct drm_driver *driver = &kirin_drm_driver;
> > > > > + struct drm_device *drm_dev;
> > > > > + struct kirin_drm_private *priv;
> > > > > + int ret;
> > > > > +
> > > > > + drm_dev = drm_dev_alloc(driver, dev);
> > > > > + if (!drm_dev)
> > > > > + return -ENOMEM;
> > > > > +
> > > > > + ret = kirin_drm_kms_init(drm_dev);
> > > > > + if (ret)
> > > > > + goto err_drm_dev_unref;
> > > > > +
> > > > > + ret = drm_dev_register(drm_dev, 0);
> > > > There is better ways to do this. See drm_drv.c for the code example.
> > >
> > > Not sure if I understood your comment here. The drm_drv.c example also
> > > calls
> > > drm_dev_register().
> >
> > This is indeed not obvious from my comments but what I wnated to say is
> > that the driver should embed drm_device in some struct,
> > maybe in "struct kirin_drm_private".
> >
> > This should also be part of the referenced example.
> >
> > I hope this clarifies it.
>
> Yeah. I was already doing those changes ;-)
>
> Something like the enclosed patch, right?
>
> Btw, I'm not sure if the error handling part is ok, as I didn't check
> what the devm stuff does at the subsystem.
>
> -
>
> On a separate question, I was unable to use the helper macros,
> as it sounds that there's no macro with this:
>
> .dumb_create= drm_gem_cma_dumb_create_internal,
>
> The existing DRM_GEM_CMA_VMAP_DRIVER_OPS uses, instead
> drm_gem_cma_dumb_create(). I'm not sure if this driver can use
> such function instead.
>From the documentation of drm_gem_cma_dumb_create_internal:
* It should not be used directly
* as their &drm_driver.dumb_create callback.
I would expect drm_gem_cma_dumb_create() to be the right choice.
So you can go ahead with DRM_GEM_CMA_VMAP_DRIVER_OPS
(I hope I am right, not the are i know much about)
> staging: hikey9xx/gpu: use drm_managed interface
>
> Use a more modern design for the driver binding logic by
> using drm_managed and getting rid of drm->dev_private.
>
> Signed-off-by: Mauro Carvalho Chehab
>
> diff --git a/drivers/staging/hikey9xx/gpu/kirin9xx_drm_drv.c
> b/drivers/staging/hikey9xx/gpu/kirin9xx_drm_drv.c
> index c7736f4d74b7..600c89605cc0 100644
> --- a/drivers/staging/hikey9xx/gpu/kirin9xx_drm_drv.c
> +++ b/drivers/staging/hikey9xx/gpu/kirin9xx_drm_drv.c
> @@ -29,12 +29,13 @@
> #include
> #include
> #include
> +#include
>
> #include "kirin9xx_drm_drv.h"
>
> static int kirin_drm_kms_cleanup(struct drm_device *dev)
> {
> - struct kirin_drm_private *priv = dev->dev_private;
> + struct kirin_drm_private *priv = to_drm_private(dev);
> static struct kirin_dc_ops const *dc_ops;
>
> if (priv->fbdev)
> @@ -45,15 +46,13 @@ static int kirin_drm_kms_cleanup(struct drm_device *dev)
> drm_kms_helper_poll_fini(dev);
> dc_ops->cleanup(dev);
> drm_mode_config_cleanup(dev);
This should also be gone when you are using
drmm_mode_config_init()
> - devm_kfree(dev->dev, priv);
> - dev->dev_private = NULL;
>
> return 0;
> }
>
> static void kirin_fbdev_output_poll_changed(struct drm_device *dev)
> {
> - struct kirin_drm_private *priv = dev->dev_private;
> + struct kirin_drm_private *priv = to_drm_private(dev);
>
> dsi_set_output_client(dev);
>
> @@ -69,18 +68,20 @@ static const struct drm_mode_config_funcs
> ki
Re: [PATCH 00/49] DRM driver for Hikey 970
Hi Mauro.
On Thu, Aug 20, 2020 at 04:06:49PM +0200, Mauro Carvalho Chehab wrote:
> Em Wed, 19 Aug 2020 19:35:58 +0200
> Sam Ravnborg escreveu:
>
> I'm already handling the other comments from your review (I'll send a
> more complete comment about them after finishing),
If you get back only on things you do not understand or do not agree on
that would be fine. The rest should be visible in the changelog on the
updated patch - no need to do extra work here.
> but I have a doubt what you meant about this:
>
> > +static int kirin_drm_bind(struct device *dev)
> > > +{
> > > + struct drm_driver *driver = &kirin_drm_driver;
> > > + struct drm_device *drm_dev;
> > > + struct kirin_drm_private *priv;
> > > + int ret;
> > > +
> > > + drm_dev = drm_dev_alloc(driver, dev);
> > > + if (!drm_dev)
> > > + return -ENOMEM;
> > > +
> > > + ret = kirin_drm_kms_init(drm_dev);
> > > + if (ret)
> > > + goto err_drm_dev_unref;
> > > +
> > > + ret = drm_dev_register(drm_dev, 0);
> > There is better ways to do this. See drm_drv.c for the code example.
>
> Not sure if I understood your comment here. The drm_drv.c example also calls
> drm_dev_register().
This is indeed not obvious from my comments but what I wnated to say is
that the driver should embed drm_device in some struct,
maybe in "struct kirin_drm_private".
This should also be part of the referenced example.
I hope this clarifies it.
Sam
>
> The only difference is that it calls it inside driver_probe(), while
> on this driver, it uses:
>
> static const struct component_master_ops kirin_drm_ops = {
> .bind = kirin_drm_bind,
> .unbind = kirin_drm_unbind,
> };
>
> static int kirin_drm_platform_probe(struct platform_device *pdev)
> {
> ...
> drm_of_component_match_add(dev, &match, compare_of, remote);
> ...
> return component_master_add_with_match(dev, &kirin_drm_ops, match);
> }
>
> Are you meaning that I should get rid of this component API and call
> kirin_drm_bind() from kirin_drm_platform_probe()?
>
> Thanks,
> Mauro
Re: [PATCH 00/49] DRM driver for Hikey 970
Hi John. > > So, IMO, the best is to keep it on staging for a while, until those > > remaining bugs gets solved. > > I'm not sure I see all of these as compelling for pushing it in via > staging. And I suspect in the process of submitting the patches for > review folks may find the cause of some of the problems you list here. There is a tendency to forget drivers in staging, and with the almost constant refactoring that happens in the drm drivers we would end up fixing this driver when a bot trigger an error. So IMO we need very good reasons to go in via staging. Sam
Re: [PATCH 49/49] dt: display: Add binds for the DPE and DSI controller for Kirin 960/970
Hi Mauro.
Some feedback in the following.
Good to see DT schma files and not .txt files - but needs a bit more
work.
Sam
On Wed, Aug 19, 2020 at 01:46:17PM +0200, Mauro Carvalho Chehab wrote:
> Add a description of the bindings used by Kirin 960/970 Display
> Serial Interface (DSI) controller and by its Display Engine (DPE).
>
> Signed-off-by: Mauro Carvalho Chehab
> ---
> .../display/hisilicon,hi3660-dpe.yaml | 99 +
> .../display/hisilicon,hi3660-dsi.yaml | 102 ++
> .../boot/dts/hisilicon/hikey970-drm.dtsi | 4 +-
> 3 files changed, 203 insertions(+), 2 deletions(-)
> create mode 100644
> Documentation/devicetree/bindings/display/hisilicon,hi3660-dpe.yaml
> create mode 100644
> Documentation/devicetree/bindings/display/hisilicon,hi3660-dsi.yaml
>
> diff --git
> a/Documentation/devicetree/bindings/display/hisilicon,hi3660-dpe.yaml
> b/Documentation/devicetree/bindings/display/hisilicon,hi3660-dpe.yaml
> new file mode 100644
> index ..074997354417
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/hisilicon,hi3660-dpe.yaml
> @@ -0,0 +1,99 @@
> +# SPDX-License-Identifier: GPL-2.0
New bindings should be dual licensed if poossible.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/hisilicon,hi3660-dpe.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: HiSilicon SPMI controller
> +
> +maintainers:
> + - Mauro Carvalho Chehab
> +
> +description: |
> + The HiSilicon Display Engine (DPE) s the display controller which grab
s/s/is/
> + image data from memory, do composition, do post image processing,
> + generate RGB timing stream and transfer to DSI.
> +
> +properties:
> + $nodename:
> +pattern: "dpe@[0-9a-f]+"
compatible will match, no need for the nodename.
> +
> + compatible:
> +enum:
> + - hisilicon,kirin960-dpe
> + - hisilicon,kirin970-dpe
> +
> + reg:
> +minItems: 1
> +
> + interrupts:
> +maxItems: 1
> +
> + clocks:
> +minItems: 1
> +description: Clocks used by the ISP and by the display
All clocks must be described.
> +
> + clock-names:
> +description: Names for the clock lines
Specify clock names here.
> +
> + dma-coherent: true
> +
> + port:
> +type: object
> +description: A port node pointing to the display output endpoint.
> +
> +
> + iommu-info:
> +type: object
> +description: IOMMU address and size to be used by GPU
> +
> +properties:
> + start-addr:
> +const: start address for IOMMU
> + size:
> +const: size of the mapped region
additionalProperties: false?
- So a DT do not use undocumented properties.
required:?
- So a DT always includes the mandatory properties
> +
> +examples:
> + - |
> +dpe: dpe@e860 {
> + compatible = "hisilicon,kirin970-dpe";
> + memory-region = <&drm_dma_reserved>;
memory-region not included in the binding.
> + reg = <0 0xE860 0 0xC>,
> +<0 0xFFF35000 0 0x1000>,
> +<0 0xFFF0A000 0 0x1000>,
> +<0 0xE8A09000 0 0x1000>,
> +<0 0xE86C 0 0x1>,
> +<0 0xFFF31000 0 0x1000>,
> +<0 0xE87FF000 0 0x1000>;
> +
> + interrupts = <0 245 4>;
> +
> + clocks = <&media1_crg HI3670_ACLK_GATE_DSS>,
> + <&media1_crg HI3670_PCLK_GATE_DSS>,
> + <&media1_crg HI3670_CLK_GATE_EDC0>,
> + <&media1_crg HI3670_CLK_GATE_LDI0>,
> + <&media1_crg HI3670_CLK_GATE_DSS_AXI_MM>,
> + <&media1_crg HI3670_PCLK_GATE_MMBUF>,
> + <&crg_ctrl HI3670_PCLK_GATE_PCTRL>;
The validation will fail as HI3670_PCLK_GATE_PCTRL is unknown.
Include the relevant header.
Use make dt_binding_check to validate the binding files.
> +
> + clock-names = "aclk_dss",
> +"pclk_dss",
> +"clk_edc0",
> +"clk_ldi0",
> +"clk_dss_axi_mm",
> +"pclk_mmbuf",
> +"pclk_pctrl";
> +
> + dma-coherent;
> +
> + port {
> +dpe_out: endpoint {
> + remote-endpoint = <&dsi_in>;
> +};
> + };
> +
> + iommu_info {
> +start-addr = <0x8000>;
> +size = <0xbfff8000>;
> + };
> +};
End file with:
...
> diff --git
> a/Documentation/devicetree/bindings/display/hisilicon,hi3660-dsi.yaml
> b/Documentation/devicetree/bindings/display/hisilicon,hi3660-dsi.yaml
> new file mode 100644
> index ..2265267fc53d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/hisilicon,hi3660-dsi.yaml
> @@ -0,0 +1,102 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/hisilicon,hi3660-dsi.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: HiSilicon SPMI controller
> +
> +maintainers:
> + - Mauro Carvalho Chehab
> +
> +description: |
> + The HiS
Re: [PATCH 00/49] DRM driver for Hikey 970
Hi Mauro. On Wed, Aug 19, 2020 at 01:45:28PM +0200, Mauro Carvalho Chehab wrote: > This patch series port the out-of-tree driver for Hikey 970 (which > should also support Hikey 960) from the official 96boards tree: > >https://github.com/96boards-hikey/linux/tree/hikey970-v4.9 > > Based on his history, this driver seems to be originally written > for Kernel 4.4, and was later ported to Kernel 4.9. The original > driver used to depend on ION (from Kernel 4.4) and had its own > implementation for FB dev API. > > As I need to preserve the original history (with has patches from > both HiSilicon and from Linaro), I'm starting from the original > patch applied there. The remaining patches are incremental, > and port this driver to work with upstream Kernel. > > This driver doesn't depend on any firmware or on any special > userspace code. It works as-is with both X11 and Wayland. > > Yet, I'm submitting it via staging due to the following reasons: > > - It depends on the LDO3 power supply, which is provided by > a regulator driver that it is currently on staging; > - Due to legal reasons, I need to preserve the authorship of > each one responsbile for each patch. So, I need to start from > the original patch from Kernel 4.4; > - There are still some problems I need to figure out how to solve: >- The adv7535 can't get EDID data. Maybe it is a timing issue, > but it requires more research to be sure about how to solve it; >- The driver only accept resolutions on a defined list, as there's > a known bug that this driver may have troubles with random > resolutions. Probably due to a bug at the pixel clock settings; >- Sometimes (at least with 1080p), it generates LDI underflow > errors, which in turn causes the DRM to stop working. That > happens for example when using gdm on Wayland and > gnome on X11; >- Probably related to the previous issue, when the monitor > suspends due to DPMS, it doesn't return back to life. > > So, IMO, the best is to keep it on staging for a while, until those > remaining bugs gets solved. > > I added this series, together with the regulator driver and > a few other patches (including a hack to fix a Kernel 5.8 > regression at WiFi ) at: > > https://gitlab.freedesktop.org/mchehab_kernel/hikey-970/-/commits/master > > > Chen Feng (1): > staging: hikey9xx: Add hisilicon DRM driver for hikey960/970 > > John Stultz (1): > staging: hikey9xx/gpu: port it to work with Kernel v4.9 > > Liwei Cai (2): > staging: hikey9xx/gpu: solve tearing issue of display > staging: hikey9xx/gpu: resolve the performance issue by interrupt > mechanism > > Mauro Carvalho Chehab (38): > staging: hikey9xx/gpu: get rid of adv7535 fork Very good - I was in my mind starting a rant why we needed a fork of this driver, but I see it gets deleted again. I do acknowledge you need to preserve history and all - but this patchset is not easy to review. Could you follow-up with a review-able set of patches as a follow-up for this? I spotted some wrong bridge handling in one patch but I do not know if this got changed in a later patch. And I lost the motivation to go looking for it. > staging: hikey9xx/gpu: rename the Kirin9xx namespace > staging: hikey9xx/gpu: get rid of kirin9xx_fbdev.c > staging: hikey9xx/gpu: get rid of some ifdefs > staging: hikey9xx/gpu: rename the config option for Kirin970 > staging: hikey9xx/gpu: change the includes to reflect upstream > staging: hikey9xx/gpu: port driver to upstream kAPIs > staging: hikey9xx/gpu: add a copy of set_reg() function there > staging: hikey9xx/gpu: get rid of ION headers > staging: hikey9xx/gpu: add support for using a reserved CMA memory > staging: hikey9xx/gpu: cleanup encoder attach logic > staging: hikey9xx/gpu: Change the logic which sets the burst mode > staging: hikey9xx/gpu: fix the DRM setting logic > staging: hikey9xx/gpu: do some code cleanups > staging: hikey9xx/gpu: use default GEM_CMA fops > staging: hikey9xx/gpu: place vblank enable/disable at the right place > staging: hikey9xx/gpu: remove an uneeded hack > staging: hikey9xx/gpu: add a possible implementation for > atomic_disable > staging: hikey9xx/gpu: register connector > staging: hikey9xx/gpu: fix driver name > staging: hikey9xx/gpu: get rid of iommu_format > staging: hikey9xx/gpu: re-work the mode validation code > staging: hikey9xx/gpu: add support for enable/disable ldo3 regulator > staging: hikey9xx/gpu: add SPMI headers > staging: hikey9xx/gpu: solve most coding style issues > staging: hikey9xx/gpu: don't use iommu code > staging: hikey9xx/gpu: add kirin9xx driver to the building system > staging: hikey9xx/gpu: get rid of typedefs > staging: hikey9xx/gpu: get rid of input/output macros > staging: hikey9xx/gpu: get rid of some unused data > staging: hikey9xx/gpu: place common definitions at kirin9xx_dpe.h > staging: hikey9xx/gpu: get rid
Re: [PATCH] dt-bindings: Whitespace clean-ups in schema files
Hi Rob. On Wed, Aug 12, 2020 at 02:36:18PM -0600, Rob Herring wrote: > Clean-up incorrect indentation, extra spaces, long lines, and missing > EOF newline in schema files. Most of the clean-ups are for list > indentation which should always be 2 spaces more than the preceding > keyword. > > Found with yamllint (which I plan to integrate into the checks). I have browsed through the patch - and there was only a few things that jumped at me. With these points considered: Acked-by: Sam Ravnborg I expect only some (few) of my points to actually results in any updates. I look forward to have the lint functionality as part of the built-in tools so we catch these things early. Sam > diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml > b/Documentation/devicetree/bindings/arm/fsl.yaml > index f63895c8ce2d..88814a2a14a5 100644 > --- a/Documentation/devicetree/bindings/arm/fsl.yaml > +++ b/Documentation/devicetree/bindings/arm/fsl.yaml > @@ -273,8 +273,8 @@ properties: >- fsl,imx6ull-14x14-evk # i.MX6 UltraLiteLite 14x14 EVK > Board >- kontron,imx6ull-n6411-som # Kontron N6411 SOM >- myir,imx6ull-mys-6ulx-eval # MYiR Tech iMX6ULL Evaluation > Board > - - toradex,colibri-imx6ull-eval# Colibri iMX6ULL > Module on Colibri Evaluation Board > - - toradex,colibri-imx6ull-wifi-eval # Colibri iMX6ULL > Wi-Fi / Bluetooth Module on Colibri Evaluation Board > + - toradex,colibri-imx6ull-eval # Colibri iMX6ULL Module > on Colibri Eval Board > + - toradex,colibri-imx6ull-wifi-eval # Colibri iMX6ULL Wi-Fi / > BT Module on Colibri Eval Board >- const: fsl,imx6ull This change looks bad as it drops the alignment with the comments below. See following patch chunck: > >- description: Kontron N6411 S Board > @@ -312,9 +312,12 @@ properties: >- toradex,colibri-imx7d # Colibri iMX7 Dual > Module >- toradex,colibri-imx7d-aster # Colibri iMX7 Dual > Module on Aster Carrier Board >- toradex,colibri-imx7d-emmc # Colibri iMX7 Dual > 1GB (eMMC) Module > - - toradex,colibri-imx7d-emmc-aster# Colibri iMX7 Dual > 1GB (eMMC) Module on Aster Carrier Board > - - toradex,colibri-imx7d-emmc-eval-v3 # Colibri iMX7 Dual > 1GB (eMMC) Module on Colibri Evaluation Board V3 > - - toradex,colibri-imx7d-eval-v3 # Colibri iMX7 Dual > Module on Colibri Evaluation Board V3 > + - toradex,colibri-imx7d-emmc-aster# Colibri iMX7 Dual > 1GB (eMMC) Module on > +# Aster Carrier > Board > diff --git > a/Documentation/devicetree/bindings/display/panel/ilitek,ili9322.yaml > b/Documentation/devicetree/bindings/display/panel/ilitek,ili9322.yaml > index 177d48c5bd97..e89c1ea62ffa 100644 > --- a/Documentation/devicetree/bindings/display/panel/ilitek,ili9322.yaml > +++ b/Documentation/devicetree/bindings/display/panel/ilitek,ili9322.yaml > @@ -25,8 +25,7 @@ properties: >compatible: > items: >- enum: > -- dlink,dir-685-panel > - > + - dlink,dir-685-panel >- const: ilitek,ili9322 > >reset-gpios: true > diff --git > a/Documentation/devicetree/bindings/display/panel/ilitek,ili9881c.yaml > b/Documentation/devicetree/bindings/display/panel/ilitek,ili9881c.yaml > index a39332276bab..76a9068a85dd 100644 > --- a/Documentation/devicetree/bindings/display/panel/ilitek,ili9881c.yaml > +++ b/Documentation/devicetree/bindings/display/panel/ilitek,ili9881c.yaml > @@ -13,8 +13,7 @@ properties: >compatible: > items: >- enum: > -- bananapi,lhr050h41 > - > + - bananapi,lhr050h41 >- const: ilitek,ili9881c > The extra lines is a simple way to indicate that here shall be added more in the future. So I like the empty line. > diff --git a/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml > b/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml > index 32e0896c6bc1..47938e372987 100644 > --- a/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml > +++ b/Documentation/devicetree/bindings/leds/backlight/qcom-wled.yaml > @@ -79,7 +79,8 @@ properties: > description: | >kHz; switching frequency. > $ref: /schemas/types.yaml#/definitions/uint32 > -enum: [ 600, 640, 685, 738, 800, 872, 960, 1066, 1200, 1371, 1600, 1920, > 2400, 3200, 4800, 9600 ] > +enum: [ 600, 640, 685, 738, 800, 872, 960, 1066, 1200, 1371, 1600, 1920, > +2400, 3200, 4800, 9600 ] > >
Re: [PATCH] docs: dt: fix broken links due to txt->yaml renames
Hi Mauro.
On Mon, May 04, 2020 at 11:30:20AM +0200, Mauro Carvalho Chehab wrote:
> There are some new broken doc links due to yaml renames
> at DT. Developers should really run:
>
> ./scripts/documentation-file-ref-check
>
> in order to solve those issues while submitting patches.
Would love if some bot could do this for me on any patches that creates
.yaml files or so.
I know I will forget this and it can be automated.
If I get a bot mail that my patch would broke a link I would
have it fixed before it hits any tree.
> This tool can even fix most of the issues with:
>
> ./scripts/documentation-file-ref-check --fix
>
> Signed-off-by: Mauro Carvalho Chehab
Patch looks good.
Acked-by: Sam Ravnborg
> ---
>
> PS.: This patch is against today's linux-next.
>
>
> .../devicetree/bindings/display/bridge/sii902x.txt | 2 +-
> .../devicetree/bindings/display/rockchip/rockchip-drm.yaml | 2 +-
> .../devicetree/bindings/net/mediatek-bluetooth.txt | 2 +-
> .../devicetree/bindings/sound/audio-graph-card.txt | 2 +-
> .../devicetree/bindings/sound/st,sti-asoc-card.txt | 2 +-
> Documentation/mips/ingenic-tcu.rst | 2 +-
> MAINTAINERS | 6 +++---
> 7 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/display/bridge/sii902x.txt
> b/Documentation/devicetree/bindings/display/bridge/sii902x.txt
> index 6e14e087c0d0..0d1db3f9da84 100644
> --- a/Documentation/devicetree/bindings/display/bridge/sii902x.txt
> +++ b/Documentation/devicetree/bindings/display/bridge/sii902x.txt
> @@ -37,7 +37,7 @@ Optional properties:
> simple-card or audio-graph-card binding. See their binding
> documents on how to describe the way the sii902x device is
> connected to the rest of the audio system:
> - Documentation/devicetree/bindings/sound/simple-card.txt
> + Documentation/devicetree/bindings/sound/simple-card.yaml
> Documentation/devicetree/bindings/sound/audio-graph-card.txt
> Note: In case of the audio-graph-card binding the used port
> index should be 3.
> diff --git
> a/Documentation/devicetree/bindings/display/rockchip/rockchip-drm.yaml
> b/Documentation/devicetree/bindings/display/rockchip/rockchip-drm.yaml
> index ec8ae742d4da..7204da5eb4c5 100644
> --- a/Documentation/devicetree/bindings/display/rockchip/rockchip-drm.yaml
> +++ b/Documentation/devicetree/bindings/display/rockchip/rockchip-drm.yaml
> @@ -24,7 +24,7 @@ properties:
> description: |
>Should contain a list of phandles pointing to display interface port
>of vop devices. vop definitions as defined in
> - Documentation/devicetree/bindings/display/rockchip/rockchip-vop.txt
> + Documentation/devicetree/bindings/display/rockchip/rockchip-vop.yaml
>
> required:
>- compatible
> diff --git a/Documentation/devicetree/bindings/net/mediatek-bluetooth.txt
> b/Documentation/devicetree/bindings/net/mediatek-bluetooth.txt
> index 219bcbd0d344..9ef5bacda8c1 100644
> --- a/Documentation/devicetree/bindings/net/mediatek-bluetooth.txt
> +++ b/Documentation/devicetree/bindings/net/mediatek-bluetooth.txt
> @@ -3,7 +3,7 @@ MediaTek SoC built-in Bluetooth Devices
>
> This device is a serial attached device to BTIF device and thus it must be a
> child node of the serial node with BTIF. The dt-bindings details for BTIF
> -device can be known via Documentation/devicetree/bindings/serial/8250.txt.
> +device can be known via Documentation/devicetree/bindings/serial/8250.yaml.
>
> Required properties:
>
> diff --git a/Documentation/devicetree/bindings/sound/audio-graph-card.txt
> b/Documentation/devicetree/bindings/sound/audio-graph-card.txt
> index 269682619a70..d5f6919a2d69 100644
> --- a/Documentation/devicetree/bindings/sound/audio-graph-card.txt
> +++ b/Documentation/devicetree/bindings/sound/audio-graph-card.txt
> @@ -5,7 +5,7 @@ It is based on common bindings for device graphs.
> see ${LINUX}/Documentation/devicetree/bindings/graph.txt
>
> Basically, Audio Graph Card property is same as Simple Card.
> -see ${LINUX}/Documentation/devicetree/bindings/sound/simple-card.txt
> +see ${LINUX}/Documentation/devicetree/bindings/sound/simple-card.yaml
>
> Below are same as Simple-Card.
>
> diff --git a/Documentation/devicetree/bindings/sound/st,sti-asoc-card.txt
> b/Documentation/devicetree/bindings/sound/st,sti-asoc-card.txt
> index 4d51f3f5ea98..a6ffcdec6f6a 100644
> --- a/Documentation/devicetree/bindings/sound/st,sti-asoc-card.txt
> +++ b/Documentation/devicetree/bindings/sound/st,sti-asoc-card.txt
> @@ -5,7 +5,7 @@ codec or external codecs.
>
Re: [PATCH 00/25] Change tty_port(standard)_install's return type
Hi Jaejoong. > Change return type for tty functions. Patch No.01 > tty: Change return type to void Adding this patch first will generate a lot of warnings until all users are updated. It is usual practice to prepare all users and then apply the infrastructure changes as the last patch. Then people will not see a lot of warnings when they build something in the middle, and I guess current stack set may also generate a few mails from the 0-day build infrastructure. > isdn: i4l: isdn_tty: Change return type to void And a nitpick on the patch description. This patch do not change any return type, but it ignore the return value og tty_part_install(). Same goes for all ramaining patches. Sam
Re: [PATCH net-next 1/2] sparc: Split BPF JIT into 32-bit and 64-bit.
On Tue, Apr 18, 2017 at 02:58:06PM -0400, David Miller wrote: > > This is in preparation for adding the 64-bit eBPF JIT. > > Signed-off-by: David S. Miller > --- > arch/sparc/net/Makefile | 2 +- > arch/sparc/net/bpf_jit.h | 68 > arch/sparc/net/bpf_jit_32.h | 68 > arch/sparc/net/bpf_jit_asm.S | 208 -- > arch/sparc/net/bpf_jit_asm_32.S | 208 ++ > arch/sparc/net/bpf_jit_asm_64.S | 1 + > arch/sparc/net/bpf_jit_comp.c| 815 > --- > arch/sparc/net/bpf_jit_comp_32.c | 815 > +++ > arch/sparc/net/bpf_jit_comp_64.c | 1 + > 9 files changed, 1094 insertions(+), 1092 deletions(-) > delete mode 100644 arch/sparc/net/bpf_jit.h > create mode 100644 arch/sparc/net/bpf_jit_32.h > delete mode 100644 arch/sparc/net/bpf_jit_asm.S > create mode 100644 arch/sparc/net/bpf_jit_asm_32.S > create mode 100644 arch/sparc/net/bpf_jit_asm_64.S > delete mode 100644 arch/sparc/net/bpf_jit_comp.c > create mode 100644 arch/sparc/net/bpf_jit_comp_32.c > create mode 100644 arch/sparc/net/bpf_jit_comp_64.c Adding options "-M -C" to git format-patch would have helped readability of this patch. As it is now we cannot see what changes were made to the varoious files when they were renamed. (I trust you have made no irrelevant but wanted to mention it anyway) Sam
Re: [PATCH] MAINTAINERS: net: Change maintainer for GRETH 10/100/1G Ethernet MAC device driver
Hi Andreas. Tak! Sam On Wed, Apr 27, 2016 at 04:46:10PM +0200, Andreas Larsson wrote: > Signed-off-by: Andreas Larsson > --- > MAINTAINERS |2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 1d5b4be..08ffebd 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -4903,7 +4903,7 @@ F: net/ipv4/gre_offload.c > F: include/net/gre.h > > GRETH 10/100/1G Ethernet MAC device driver > -M: Kristoffer Glembo > +M: Andreas Larsson > L: netdev@vger.kernel.org > S: Maintained > F: drivers/net/ethernet/aeroflex/ > -- > 1.7.10.4 >
Re: [patch 2.6.25-rc3] smc91x section fix
On Sun, Feb 24, 2008 at 10:33:12PM -0800, David Brownell wrote: > On Sunday 24 February 2008, Sam Ravnborg wrote: > > > From a quick look this is wrong. > > smc_drv_probe is assined the .probe member so it is used during > > hotplug and thus should be __devinit. > > Likewise smc_probe is used by smc_drv_probe and thus smc_probe > > should be __devinit too. > > Thing is, with only rare exceptions, devices on the platform > bus are *NOT* hotpluggable. So using __devinit/__devexit and > friends adds up to no more than a waste of I-space. It was a quick look - thanks for the explanation. Sam -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 2.6.25-rc3] smc91x section fix
On Sun, Feb 24, 2008 at 07:34:11PM -0800, David Brownell wrote:
> Section fixup:
>
> WARNING: drivers/net/built-in.o(.text+0x1a2c): Section mismatch
> in reference from the function smc_drv_probe()
> to the function .init.text:smc_probe()
> The function smc_drv_probe() references
> the function __init smc_probe().
> This is often because smc_drv_probe lacks a __init
> annotation or the annotation of smc_probe is wrong.
>
> Signed-off-by: David Brownell <[EMAIL PROTECTED]>
> ---
> drivers/net/smc91x.c |2 +-
> 1 files changed, 1 insertion(+), 1 deletion(-)
>
> --- a/drivers/net/smc91x.c2008-02-24 18:50:32.0 -0800
> +++ b/drivers/net/smc91x.c2008-02-24 19:16:29.0 -0800
> @@ -2121,7 +2121,7 @@ static void smc_release_datacs(struct pl
> * 0 --> there is a device
> * anything else, error
> */
> -static int smc_drv_probe(struct platform_device *pdev)
> +static int __init smc_drv_probe(struct platform_device *pdev)
> {
> struct net_device *ndev;
> struct resource *res, *ires;
>From a quick look this is wrong.
smc_drv_probe is assined the .probe member so it is used during
hotplug and thus should be __devinit.
Likewise smc_probe is used by smc_drv_probe and thus smc_probe
should be __devinit too.
So something like the appended. On my way out of the door so
not even build tested!
Sam
diff --git a/drivers/net/smc91x.c b/drivers/net/smc91x.c
index 4020e9e..640bca5 100644
--- a/drivers/net/smc91x.c
+++ b/drivers/net/smc91x.c
@@ -1775,7 +1775,7 @@ static int __init smc_findirq(void __iomem *ioaddr)
* o actually GRAB the irq.
* o GRAB the region
*/
-static int __init smc_probe(struct net_device *dev, void __iomem *ioaddr,
+static int __devinit smc_probe(struct net_device *dev, void __iomem *ioaddr,
unsigned long irq_flags)
{
struct smc_local *lp = netdev_priv(dev);
@@ -2121,7 +2121,7 @@ static void smc_release_datacs(struct platform_device
*pdev, struct net_device *
* 0 --> there is a device
* anything else, error
*/
-static int smc_drv_probe(struct platform_device *pdev)
+static int __devinit smc_drv_probe(struct platform_device *pdev)
{
struct net_device *ndev;
struct resource *res, *ires;
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 03/08] [TIPC]: Eliminate "sparse" warning in socket code
On Fri, Feb 22, 2008 at 11:59:12AM -0800, Stephens, Allan wrote: > This patch eliminates warnings about a pair of undeclared symbols. Hi Allan. Last I looked there where ~90 sparse warnings for tipc. I assume you are aware and that they are getting fixed. Sam - who would like to see an almost sparse clean kernel -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
PowerPC toolchain for x86 [Was: pci_device_id cleanups]
On Wed, Feb 20, 2008 at 02:27:19PM +0100, Jonas Bonn wrote: > Sam Ravnborg wrote: > >On Wed, Feb 20, 2008 at 01:53:36PM +0100, Jonas Bonn wrote: > >>The PCI_DEVICE_TABLE patch I sent earlier doesn't necessarily make > >>much sense by itself... here is a set of patches that apply > >>this macro, in turn moving a lot of this data into __devinitconst > >>which is discardable in certain situations. > >>Hopefully the benefit of this approach is a bit clearer now. > >[shorter lines please..] > > Sorry... > > > > >Can you please confirm that this does not break powerpc (64 bit) > >as they have troubles with the constification.. > > I do not have access to any PowerPC machine... Olof Johansson built the > tree I posted earlier on PowerPC; there's nothing really new here except > the wrapping of the definition in a macro. And you added const and a specific section. Exactly what could break on PowerPC. To do the build break check is easy. Google for "crosstool" and build your own powerpc toolchain. Andrew has something precompiled somewhere but I lost the link. Sam -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: pci_device_id cleanups
On Wed, Feb 20, 2008 at 01:53:36PM +0100, Jonas Bonn wrote: > > The PCI_DEVICE_TABLE patch I sent earlier doesn't necessarily make > much sense by itself... here is a set of patches that apply > this macro, in turn moving a lot of this data into __devinitconst > which is discardable in certain situations. > Hopefully the benefit of this approach is a bit clearer now. [shorter lines please..] Can you please confirm that this does not break powerpc (64 bit) as they have troubles with the constification.. Sam -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATHCH 0/16] ServerEngines 10Gb NIC driver
On Sun, Feb 17, 2008 at 02:25:36PM +0100, Sam Ravnborg wrote: > On Sat, Feb 16, 2008 at 07:51:13PM -0800, Subbu Seetharaman wrote: > > Patch 0/16 and 15/16 of this series are getting dropped by the > > spam filter. I am trying to get them across with changes > > that will please the spam filter. Sorry about the inconvenience. > > Below is the 0/16 of the series. > > > > -- > > Hi, > > > > > > I am sending a patch for network driver for ServerEngines 10Gb Network > > adapter for review. This patch includes the network driver and the OS > > neutral code that implements the interactions between the host drivers and > > the adapter. The adapter is a dual function device with network and storage > > functions. The network driver is a regular NIC driver. > > > > The low level library that manages the interaction with the adapter is > > common to both storage and network drivers and hence is organized under the > > directory drivers/message/beclib. The storage driver is not part of this > > patch and will be submitted after this review. > > > > This patch is made against 2.6.24.2 version of the kernel source. > > Hi Subbu. > > A few points about your submission (not your code as I have not looked at it). > - Use descriptive subjects - and not the same subject for all patches > - Include in each patch a diffstat - below the '^---' marker so > it is easy to spot what files are touched but so that the diffstat > does not get included in the changelog when applied. > - Do not break the build. Adding Makefile/Kconfig bits as the last patch > usually does the trick here. > - If you have generic funtionality add that first and then add what depnds on > it > later. > > Sam I just tried scripts/checkpatch.pl - it said: total: 250 errors, 837 warnings, 31889 lines checked That was for the full patch-set. I suggest you look into these. The goal is not to get down on 0/0 but close with some common sense applied. Sam -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATHCH 0/16] ServerEngines 10Gb NIC driver
On Sat, Feb 16, 2008 at 07:51:13PM -0800, Subbu Seetharaman wrote: > Patch 0/16 and 15/16 of this series are getting dropped by the > spam filter. I am trying to get them across with changes > that will please the spam filter. Sorry about the inconvenience. > Below is the 0/16 of the series. > > -- > Hi, > > > I am sending a patch for network driver for ServerEngines 10Gb Network > adapter for review. This patch includes the network driver and the OS > neutral code that implements the interactions between the host drivers and > the adapter. The adapter is a dual function device with network and storage > functions. The network driver is a regular NIC driver. > > The low level library that manages the interaction with the adapter is common > to both storage and network drivers and hence is organized under the > directory drivers/message/beclib. The storage driver is not part of this > patch and will be submitted after this review. > > This patch is made against 2.6.24.2 version of the kernel source. Hi Subbu. A few points about your submission (not your code as I have not looked at it). - Use descriptive subjects - and not the same subject for all patches - Include in each patch a diffstat - below the '^---' marker so it is easy to spot what files are touched but so that the diffstat does not get included in the changelog when applied. - Do not break the build. Adding Makefile/Kconfig bits as the last patch usually does the trick here. - If you have generic funtionality add that first and then add what depnds on it later. Sam -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] drivers/base: export gpl (un)register_memory_notifier
On Mon, Feb 11, 2008 at 04:57:06PM +0100, Jan-Bernd Themann wrote: > Drivers like eHEA need memory notifiers in order to > update their internal DMA memory map when memory is added > to or removed from the system. Can you please add proper kernel-doc formatted comments when you export a symbol so others can get a clue what the exported function is supposed to do. Sam -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] isdn: fix section mismatch warnings from hisax_cs_setup_card
Fix the following warnings:
WARNING: drivers/isdn/hisax/built-in.o(.text+0x722): Section mismatch in
reference from the function hisax_cs_setup_card() to the function
.devinit.text:setup_teles3()
WARNING: drivers/isdn/hisax/built-in.o(.text+0x72c): Section mismatch in
reference from the function hisax_cs_setup_card() to the function
.devinit.text:setup_s0box()
WARNING: drivers/isdn/hisax/built-in.o(.text+0x736): Section mismatch in
reference from the function hisax_cs_setup_card() to the function
.devinit.text:setup_telespci()
WARNING: drivers/isdn/hisax/built-in.o(.text+0x747): Section mismatch in
reference from the function hisax_cs_setup_card() to the function
.devinit.text:setup_avm_pcipnp()
WARNING: drivers/isdn/hisax/built-in.o(.text+0x74e): Section mismatch in
reference from the function hisax_cs_setup_card() to the function
.devinit.text:setup_elsa()
WARNING: drivers/isdn/hisax/built-in.o(.text+0x755): Section mismatch in
reference from the function hisax_cs_setup_card() to the function
.devinit.text:setup_diva()
WARNING: drivers/isdn/hisax/built-in.o(.text+0x75c): Section mismatch in
reference from the function hisax_cs_setup_card() to the function
.devinit.text:setup_sedlbauer()
WARNING: drivers/isdn/hisax/built-in.o(.text+0x763): Section mismatch in
reference from the function hisax_cs_setup_card() to the function
.devinit.text:setup_netjet_s()
WARNING: drivers/isdn/hisax/built-in.o(.text+0x76a): Section mismatch in
reference from the function hisax_cs_setup_card() to the function
.devinit.text:setup_hfcpci()
WARNING: drivers/isdn/hisax/built-in.o(.text+0x771): Section mismatch in
reference from the function hisax_cs_setup_card() to the function
.devinit.text:setup_hfcsx()
WARNING: drivers/isdn/hisax/built-in.o(.text+0x778): Section mismatch in
reference from the function hisax_cs_setup_card() to the function
.devinit.text:setup_niccy()
WARNING: drivers/isdn/hisax/built-in.o(.text+0x77f): Section mismatch in
reference from the function hisax_cs_setup_card() to the function
.devinit.text:setup_bkm_a4t()
WARNING: drivers/isdn/hisax/built-in.o(.text+0x786): Section mismatch in
reference from the function hisax_cs_setup_card() to the function
.devinit.text:setup_sct_quadro()
WARNING: drivers/isdn/hisax/built-in.o(.text+0x78d): Section mismatch in
reference from the function hisax_cs_setup_card() to the function
.devinit.text:setup_gazel()
WARNING: drivers/isdn/hisax/built-in.o(.text+0x794): Section mismatch in
reference from the function hisax_cs_setup_card() to the function
.devinit.text:setup_w6692()
WARNING: drivers/isdn/hisax/built-in.o(.text+0x79b): Section mismatch in
reference from the function hisax_cs_setup_card() to the function
.devinit.text:setup_netjet_u()
WARNING: drivers/isdn/hisax/built-in.o(.text+0x7a2): Section mismatch in
reference from the function hisax_cs_setup_card() to the function
.devinit.text:setup_enternow_pci()
checkcard() are the only user of hisax_cs_setup_card().
And checkcard is only used during init or when hot plugging
ISDN devices. So annotate hisax_cs_setup_card() with __devinit.
checkcard() is used by exported functions so it cannot be
annotated __devinit. Annotate it with __ref so modpost
ignore references to _devinit section.
Signed-off-by: Sam Ravnborg <[EMAIL PROTECTED]>
Acked-by: Karsten Keil <[EMAIL PROTECTED]>
Cc: Jeff Garzik <[EMAIL PROTECTED]>
Cc: David Miller <[EMAIL PROTECTED]>
---
---
drivers/isdn/hisax/config.c |7 +--
1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/isdn/hisax/config.c b/drivers/isdn/hisax/config.c
index 97097ef..a0ee43c 100644
--- a/drivers/isdn/hisax/config.c
+++ b/drivers/isdn/hisax/config.c
@@ -847,7 +847,7 @@ static int init_card(struct IsdnCardState *cs)
return 3;
}
-static int hisax_cs_setup_card(struct IsdnCard *card)
+static int __devinit hisax_cs_setup_card(struct IsdnCard *card)
{
int ret;
@@ -1166,7 +1166,10 @@ outf_cs:
return 0;
}
-static int checkcard(int cardnr, char *id, int *busy_flag, struct module
*lockowner)
+/* Used from an exported function but calls __devinit functions.
+ * Tell modpost not to warn (__ref)
+ */
+static int __ref checkcard(int cardnr, char *id, int *busy_flag, struct module
*lockowner)
{
int ret;
struct IsdnCard *card = cards + cardnr;
--
1.5.4.rc3.14.g44397
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] isdn: fix section mismatch warning for ISACVer
Fix following warnings:
WARNING: drivers/isdn/hisax/built-in.o(.text+0x19723): Section mismatch in
reference from the function ISACVersion() to the variable .devinit.data:ISACVer
WARNING: drivers/isdn/hisax/built-in.o(.text+0x2005b): Section mismatch in
reference from the function setup_avm_a1_pcmcia() to the function
.devinit.text:setup_isac()
ISACVer were only used from function annotated __devinit
so add same annotation to ISACVer.
One af the fererencing functions missed __devinit so add it
and kill an additional warning.
Signed-off-by: Sam Ravnborg <[EMAIL PROTECTED]>
Acked-by: Karsten Keil <[EMAIL PROTECTED]>
Cc: Jeff Garzik <[EMAIL PROTECTED]>
Cc: David Miller <[EMAIL PROTECTED]>
---
drivers/isdn/hisax/avm_a1p.c |3 +--
drivers/isdn/hisax/isac.c|3 +--
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/isdn/hisax/avm_a1p.c b/drivers/isdn/hisax/avm_a1p.c
index c87fa3f..3039c6d 100644
--- a/drivers/isdn/hisax/avm_a1p.c
+++ b/drivers/isdn/hisax/avm_a1p.c
@@ -213,8 +213,7 @@ AVM_card_msg(struct IsdnCardState *cs, int mt, void *arg)
return 0;
}
-int
-setup_avm_a1_pcmcia(struct IsdnCard *card)
+int __devinit setup_avm_a1_pcmcia(struct IsdnCard *card)
{
u_char model, vers;
struct IsdnCardState *cs = card->cs;
diff --git a/drivers/isdn/hisax/isac.c b/drivers/isdn/hisax/isac.c
index 4e9f238..6ae9dde 100644
--- a/drivers/isdn/hisax/isac.c
+++ b/drivers/isdn/hisax/isac.c
@@ -27,8 +27,7 @@ static char *ISACVer[] __devinitdata =
{"2086/2186 V1.1", "2085 B1", "2085 B2",
"2085 V2.3"};
-void
-ISACVersion(struct IsdnCardState *cs, char *s)
+void __devinit ISACVersion(struct IsdnCardState *cs, char *s)
{
int val;
--
1.5.4.rc3.14.g44397
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] isdn: fix section mismatch warning in hfc_sx.c
Fix the following warning:
WARNING: drivers/isdn/hisax/built-in.o(.text+0x35818): Section mismatch in
reference from the function hfcsx_card_msg() to the function
.devinit.text:inithfcsx()
hfcsx_card_msg() may be called outside __devinit context.
Following the program logic is looks like the CARD_INIT branch
will only be taken under __devinit context but to be consistent
remove the __devinit annotation of inithfcsx() so we
do not mix non-__devinit and __devinit code.
Signed-off-by: Sam Ravnborg <[EMAIL PROTECTED]>
Acked-by: Karsten Keil <[EMAIL PROTECTED]>
Cc: Jeff Garzik <[EMAIL PROTECTED]>
Cc: David Miller <[EMAIL PROTECTED]>
---
drivers/isdn/hisax/hfc_sx.c |3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/drivers/isdn/hisax/hfc_sx.c b/drivers/isdn/hisax/hfc_sx.c
index 4fd09d2..05482d2 100644
--- a/drivers/isdn/hisax/hfc_sx.c
+++ b/drivers/isdn/hisax/hfc_sx.c
@@ -1330,8 +1330,7 @@ hfcsx_bh(struct work_struct *work)
//
/* called for card init message */
//
-static void __devinit
-inithfcsx(struct IsdnCardState *cs)
+static void inithfcsx(struct IsdnCardState *cs)
{
cs->setstack_d = setstack_hfcsx;
cs->BC_Send_Data = &hfcsx_send_data;
--
1.5.4.rc3.14.g44397
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] isdn: fix section mismatch warnings in isac.c and isar.c
Fix the following warnings:
WARNING: drivers/isdn/hisax/built-in.o(.text+0x1b276): Section mismatch in
reference from the function inithscxisac() to the function
.devinit.text:clear_pending_isac_ints()
WARNING: drivers/isdn/hisax/built-in.o(.text+0x1b286): Section mismatch in
reference from the function inithscxisac() to the function
.devinit.text:initisac()
WARNING: drivers/isdn/hisax/built-in.o(.text+0x1fec7): Section mismatch in
reference from the function AVM_card_msg() to the function
.devinit.text:clear_pending_isac_ints()
WARNING: drivers/isdn/hisax/built-in.o(.text+0x21669): Section mismatch in
reference from the function AVM_card_msg() to the function
.devinit.text:clear_pending_isac_ints()
WARNING: drivers/isdn/hisax/built-in.o(.text+0x21671): Section mismatch in
reference from the function AVM_card_msg() to the function
.devinit.text:initisac()
WARNING: drivers/isdn/hisax/built-in.o(.text+0x2991e): Section mismatch in
reference from the function Sedl_card_msg() to the function
.devinit.text:clear_pending_isac_ints()
WARNING: drivers/isdn/hisax/built-in.o(.text+0x29936): Section mismatch in
reference from the function Sedl_card_msg() to the function
.devinit.text:initisac()
WARNING: drivers/isdn/hisax/built-in.o(.text+0x2993e): Section mismatch in
reference from the function Sedl_card_msg() to the function
.devinit.text:initisar()
WARNING: drivers/isdn/hisax/built-in.o(.text+0x2e026): Section mismatch in
reference from the function NETjet_S_card_msg() to the function
.devinit.text:clear_pending_isac_ints()
WARNING: drivers/isdn/hisax/built-in.o(.text+0x2e02e): Section mismatch in
reference from the function NETjet_S_card_msg() to the function
.devinit.text:initisac()
WARNING: drivers/isdn/hisax/built-in.o(.text+0x37813): Section mismatch in
reference from the function BKM_card_msg() to the function
.devinit.text:clear_pending_isac_ints()
WARNING: drivers/isdn/hisax/built-in.o(.text+0x37823): Section mismatch in
reference from the function BKM_card_msg() to the function
.devinit.text:initisac()
initisar(), initisac() and clear_pending_isac_ints()
were all used via a cardmsg fnction - which may be called
ouside __devinit context.
So remove the bogus __devinit annotation of the
above three functions to fix the warnings.
Signed-off-by: Sam Ravnborg <[EMAIL PROTECTED]>
Acked-by: Karsten Keil <[EMAIL PROTECTED]>
Cc: Jeff Garzik <[EMAIL PROTECTED]>
Cc: David Miller <[EMAIL PROTECTED]>
---
drivers/isdn/hisax/isac.c |6 ++
drivers/isdn/hisax/isar.c |3 +--
2 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/isdn/hisax/isac.c b/drivers/isdn/hisax/isac.c
index 6ae9dde..07b1673 100644
--- a/drivers/isdn/hisax/isac.c
+++ b/drivers/isdn/hisax/isac.c
@@ -615,8 +615,7 @@ dbusy_timer_handler(struct IsdnCardState *cs)
}
}
-void __devinit
-initisac(struct IsdnCardState *cs)
+void initisac(struct IsdnCardState *cs)
{
cs->setstack_d = setstack_isac;
cs->DC_Close = DC_Close_isac;
@@ -647,8 +646,7 @@ initisac(struct IsdnCardState *cs)
cs->writeisac(cs, ISAC_MASK, 0x0);
}
-void __devinit
-clear_pending_isac_ints(struct IsdnCardState *cs)
+void clear_pending_isac_ints(struct IsdnCardState *cs)
{
int val, eval;
diff --git a/drivers/isdn/hisax/isar.c b/drivers/isdn/hisax/isar.c
index c547a66..bfeb9b6 100644
--- a/drivers/isdn/hisax/isar.c
+++ b/drivers/isdn/hisax/isar.c
@@ -1894,8 +1894,7 @@ isar_auxcmd(struct IsdnCardState *cs, isdn_ctrl *ic) {
return(0);
}
-void __devinit
-initisar(struct IsdnCardState *cs)
+void initisar(struct IsdnCardState *cs)
{
cs->bcs[0].BC_SetStack = setstack_isar;
cs->bcs[1].BC_SetStack = setstack_isar;
--
1.5.4.rc3.14.g44397
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 1/7] typhoon section fix
On Fri, Feb 08, 2008 at 01:21:57PM -0500, David Dillow wrote:
>
> On Fri, 2008-02-08 at 09:52 -0800, Andrew Morton wrote:
> > On Fri, 08 Feb 2008 08:59:10 -0500 David Dillow <[EMAIL PROTECTED]> wrote:
> > > On Fri, 2008-02-08 at 03:11 -0800, [EMAIL PROTECTED] wrote:
> > > > From: Andrew Morton <[EMAIL PROTECTED]>
> > > >
> > > > gcc-3.4.4 on powerpc:
> > > >
> > > > drivers/net/typhoon.c:137: error: version causes a section type conflict
> > > >
> > > > Cc: Jeff Garzik <[EMAIL PROTECTED]>
> > > > Cc: Sam Ravnborg <[EMAIL PROTECTED]>
> > > > Signed-off-by: Andrew Morton <[EMAIL PROTECTED]>
> > >
> > > > -static const char version[] __devinitdata =
> > > > +static char version[] __devinitdata =
> > > > "typhoon.c: version " DRV_MODULE_VERSION " (" DRV_MODULE_RELDATE
> > > > ")\n";
> > >
> > > Wouldn't going to __devinitconst be better? I'll try to whip up a patch
> > > and test-compile it.
> >
> > Sam told me that doesn't work right, that this approach is the one to use
> > and, iirc, that __devinitcont and friends will be removed.
> >
> > I'm not sure why, actually. I think I missed that dicussion.
After introducing dedicated sections for __devinit, __devinitdata & friends
and introducing __devinitconst for const data we started to see
section type conflict _errors_ emitted from gcc for certain architectures.
64bit powerpc and ia64 being two of them.
That was rooted down by Al to the following:
===
; cat >a.c <<'EOF'
const char foo[] __attribute__ ((__section__(".blah"))) = "";
const char * const bar __attribute__((__section__(".blah"))) = "";
EOF
; gcc -m32 -S a.c
; gcc -m64 -S a.c
a.c:2: error: bar causes a section type conflict
;
===
Which tells us that the same data in some cases are put in a readonly
section and in other cases not.
And when we force data for tow different variables where gcc thinks one is
const and the other is not const to the same section gcc errros
out with the "section type conflict" error.
And this is becoming increasingly visible when people start to constify
the data all around and do test build on x86 64bit that does not exhibit
this behaviour.
The rationale behind the increased constification of data is to get
improved use of the DEBUG_RODATA thing.
Another good reason s that this is a good way to let gcc catch accidental
writes to data that otherwise should have been read-only.
But if we advocate for constification of data than we should have
a reliable way to detect the section type conflict errors on x86
(at least 64 bit) which we do not have. Missing that will casue us to see
an increasing flood of error reports from our power pc and ia64 builders.
The rationale behind __devinitconst was to create a dedicated section
for data marked read-only by gcc and thus avoiding the
section type conflict.
But as shown by the above code snippet this is not easy.
For x86 (32 + 64bit) both variables end up in READ-ONLY section.
For Powerpc 64 bit the variable bar end up in a read-write section.
And I see no way to check this for a x86 build so we warn early and
with the most widely used tool-chain.
So do we have other options that to drop the constification and thus
dropping the __devinitconst and the other __*const annotations - I think not :-(
Sam
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] 2.6.24-mm1 section type conflict cleanup
On Mon, Feb 04, 2008 at 09:52:23PM +0530, Kamalesh Babulal wrote: > Hi Andrew, > > The 2.6.24-mm1 kernel build fails at many places with section type > conflict build error. What arch? We have troubles with powerpc as pointed out by Al in another thread. Sam -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] sis190: fix compile error section type conflict
On Sat, Feb 02, 2008 at 03:34:58PM +0100, Francois Romieu wrote: > Li Zefan <[EMAIL PROTECTED]> : > > Fix the following compile error: > > I am not sure that it is the right fix. Please read the archive of > this week on l-k. > > I'll submit the patch below to Jeff once I have tested it with real > hardware (sunday evening entertainment). Sam, is it ok to add your > s-o-b to it ? No - a s-o-b document the path of the patch. And it will not hit Jeff via me. Although I know I said ths was to way to do it. But you can add my: Acked-by: Sam Ravnborg <[EMAIL PROTECTED]> If I posted the patch myself I do not remember - but then the s-o-b would be correct. Sam -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUILD FAILURE]2.6.24-git10 section type conflict
On Fri, Feb 01, 2008 at 11:56:27PM +0530, Sudhir Kumar wrote: > Hi All, > I found the following kernel build failure for 2.6.24-git10 > on my machine: > > CC [M] drivers/scsi/lpfc/lpfc_attr.o > drivers/net/sis190.c:329: error: sis190_pci_tbl causes a section type > conflict > make[2]: *** [drivers/net/sis190.o] Error 1 > make[1]: *** [drivers/net] Error 2 > make[1]: *** Waiting for unfinished jobs > CC [M] drivers/scsi/lpfc/lpfc_vport.o Known issue with fix already pushed upstream. A workaround is to delete __devinitdata from the corresponding .c (sis190.c) file. Sam -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
netdev share of section mismatch warnings...
Can we please get the following warnings fixed in mainline soonish. I get the below list with a x86 64bit allyesconfig build and I expect anyone to see roughly the same list. It would be great to say that both net/ and drivers/net/ were warning free in this respect. If you have questions let me know and I will try to help out. Sam WARNING: drivers/net/built-in.o(.text+0x42e93): Section mismatch in reference from the function t3_io_slot_reset() to the function .devinit.text:t3_prep_adapter() The function t3_io_slot_reset() references the function __devinit t3_prep_adapter(). This is often because t3_io_slot_reset lacks a __devinit annotation or the annotation of t3_prep_adapter is wrong. WARNING: drivers/net/built-in.o(.text+0x89927): Section mismatch in reference from the function sis190_get_mac_addr() to the function .devinit.text:sis190_get_mac_addr_from_apc() The function sis190_get_mac_addr() references the function __devinit sis190_get_mac_addr_from_apc(). This is often because sis190_get_mac_addr lacks a __devinit annotation or the annotation of sis190_get_mac_addr_from_apc is wrong. WARNING: drivers/net/built-in.o(.text+0x89934): Section mismatch in reference from the function sis190_get_mac_addr() to the function .devinit.text:sis190_get_mac_addr_from_eeprom() The function sis190_get_mac_addr() references the function __devinit sis190_get_mac_addr_from_eeprom(). This is often because sis190_get_mac_addr lacks a __devinit annotation or the annotation of sis190_get_mac_addr_from_eeprom is wrong. WARNING: drivers/net/built-in.o(.text+0x14866b): Section mismatch in reference from the function mlx4_init_icm() to the function .devinit.text:mlx4_init_cmpt_table() The function mlx4_init_icm() references the function __devinit mlx4_init_cmpt_table(). This is often because mlx4_init_icm lacks a __devinit annotation or the annotation of mlx4_init_cmpt_table is wrong. WARNING: drivers/net/built-in.o(.text+0x148c19): Section mismatch in reference from the function mlx4_init_hca() to the function .devinit.text:mlx4_load_fw() The function mlx4_init_hca() references the function __devinit mlx4_load_fw(). This is often because mlx4_init_hca lacks a __devinit annotation or the annotation of mlx4_load_fw is wrong. WARNING: drivers/net/built-in.o(.text+0x1494bd): Section mismatch in reference from the function __mlx4_init_one() to the function .devinit.text:mlx4_enable_msi_x() The function __mlx4_init_one() references the function __devinit mlx4_enable_msi_x(). This is often because __mlx4_init_one lacks a __devinit annotation or the annotation of mlx4_enable_msi_x is wrong. WARNING: drivers/net/built-in.o(.text+0x14a77e): Section mismatch in reference from the function mlx4_init_mr_table() to the function .devinit.text:mlx4_buddy_init() The function mlx4_init_mr_table() references the function __devinit mlx4_buddy_init(). This is often because mlx4_init_mr_table lacks a __devinit annotation or the annotation of mlx4_buddy_init is wrong. WARNING: drivers/net/built-in.o(.text+0x1500a1): Section mismatch in reference from the function olympic_open() to the function .devinit.text:olympic_init() The function olympic_open() references the function __devinit olympic_init(). This is often because olympic_open lacks a __devinit annotation or the annotation of olympic_init is wrong. WARNING: drivers/net/built-in.o(.data+0x54478): Section mismatch in reference from the variable ath5k_pci_drv_id to the variable .devinit.data:ath5k_pci_id_table The variable ath5k_pci_drv_id references the variable __devinitdata ath5k_pci_id_table If the reference is valid then annotate the variable with __init* (see linux/init.h) or name the variable: *driver, *_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console, WARNING: drivers/net/built-in.o(.data+0x54480): Section mismatch in reference from the variable ath5k_pci_drv_id to the function .devinit.text:ath5k_pci_probe() The variable ath5k_pci_drv_id references the function __devinit ath5k_pci_probe() If the reference is valid then annotate the variable with __init* (see linux/init.h) or name the variable: *driver, *_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console, WARNING: drivers/net/built-in.o(.data+0x54488): Section mismatch in reference from the variable ath5k_pci_drv_id to the function .devexit.text:ath5k_pci_remove() The variable ath5k_pci_drv_id references the function __devexit ath5k_pci_remove() If the reference is valid then annotate the variable with __exit* (see linux/init.h) or name the variable: *driver, *_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console, -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] ieee80211: fix section mismatch warning
Fix the following warnings:
WARNING: net/built-in.o(.init.text+0xd6c0): Section mismatch in reference from
the function ieee80211_init() to the function .exit.text:rc80211_simple_exit()
WARNING: net/built-in.o(.init.text+0xd6c5): Section mismatch in reference from
the function ieee80211_init() to the function .exit.text:rc80211_pid_exit()
The fix was simple - I just did as modpost told me and removed the
wrong __exit annotation of rc80211_simple_exit and rc80211_pid_exit.
Signed-off-by: Sam Ravnborg <[EMAIL PROTECTED]>
Cc: Johannes Berg <[EMAIL PROTECTED]>
Cc: John W. Linville <[EMAIL PROTECTED]>
Cc: David S. Miller <[EMAIL PROTECTED]>
---
With this patch my allyesconfig build on x86 (64 bit)
is section mismatch clean in net/
Sam
diff --git a/net/mac80211/rc80211_pid_algo.c b/net/mac80211/rc80211_pid_algo.c
index 554c4ba..c339571 100644
--- a/net/mac80211/rc80211_pid_algo.c
+++ b/net/mac80211/rc80211_pid_algo.c
@@ -538,7 +538,7 @@ int __init rc80211_pid_init(void)
return ieee80211_rate_control_register(&mac80211_rcpid);
}
-void __exit rc80211_pid_exit(void)
+void rc80211_pid_exit(void)
{
ieee80211_rate_control_unregister(&mac80211_rcpid);
}
diff --git a/net/mac80211/rc80211_simple.c b/net/mac80211/rc80211_simple.c
index 934676d..9a78b11 100644
--- a/net/mac80211/rc80211_simple.c
+++ b/net/mac80211/rc80211_simple.c
@@ -389,7 +389,7 @@ int __init rc80211_simple_init(void)
return ieee80211_rate_control_register(&mac80211_rcsimple);
}
-void __exit rc80211_simple_exit(void)
+void rc80211_simple_exit(void)
{
ieee80211_rate_control_unregister(&mac80211_rcsimple);
}
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [git patches] net driver fixes
On Wed, Jan 30, 2008 at 11:47:11PM +0100, Francois Romieu wrote:
> Sam Ravnborg <[EMAIL PROTECTED]> :
> [...]
> > > -static struct pci_device_id sis190_pci_tbl[] __devinitdata = {
> > > +static struct pci_device_id sis190_pci_tbl[] = {
> > > { PCI_DEVICE(PCI_VENDOR_ID_SI, 0x0190), 0, 0, 0 },
> > > { PCI_DEVICE(PCI_VENDOR_ID_SI, 0x0191), 0, 0, 1 },
> > > { 0, },
> >
> > The __devinitdata is OK, it is the following _devinitdata that had
> > to be _devinitconst.
>
> Strangely enough, removing the devinitdata from the sis190_pci_tbl
> silents the error message here. Do you have an explanation ?
gcc compalins if you add const and non-const data to the same section
which is the case in this driver.
The bug are exposed now where __devinitdata are no longer an empty define.
Sam
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUILD FAILURE] 2.6.24-git7 section type conflict at various drivers on powerpc
On Wed, Jan 30, 2008 at 09:49:59PM +0530, Kamalesh Babulal wrote: > Hi, > > Following are the different build failure with 2.6.24-git7 kernel on the > powerpc > > drivers/net/typhoon.c:181: error: typhoon_card_info causes a section type > conflict > make[2]: *** [drivers/net/typhoon.o] Error 1 > > drivers/net/natsemi.c:259: error: natsemi_pci_info causes a section type > conflict > make[2]: *** [drivers/net/natsemi.o] Error 1 > > drivers/net/bnx2.c:95: error: board_info causes a section type conflict > make[2]: *** [drivers/net/bnx2.o] Error 1 > > drivers/net/via-velocity.c:454: error: velocity_id_table causes a section > type conflict > make[2]: *** [drivers/net/via-velocity.o] Error 1 A quick look told me that they are all caused by const data annotated with __devinitdata. Try replacing all annotations of const variables from __devinitdata to __devinitconst. Sam -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [SIS190] Constify data marked as __devinitdata
On Wed, Jan 30, 2008 at 02:31:05PM +0100, Jan Engelhardt wrote:
>
> On Jan 30 2008 12:25, Sam Ravnborg wrote:
> >
> >We have just introduced __initconst, __cpuinitconst, __meminitconst
> >for const data.
> >So the patch is wrong.
>
> Oh joy, more tags. Is it actually possible to combine const
> with __devinitconst now?
>
> static const uint16_t foo[] __devinitconst = { ... };
Yes, try it.
Sam
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [SIS190] Use _devinitconst for const data
On Wed, Jan 30, 2008 at 12:57:16PM +0100, Jonas Bonn wrote:
> This fixes build error as gcc complains about a "section type conflict"
> due to the mixing of const and non-const data in same section.
> ---
> drivers/net/sis190.c |4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/sis190.c b/drivers/net/sis190.c
> index b570402..f84c02e 100644
> --- a/drivers/net/sis190.c
> +++ b/drivers/net/sis190.c
> @@ -326,7 +326,7 @@ static const struct {
> { "SiS 191 PCI Gigabit Ethernet adapter" },
> };
>
> -static struct pci_device_id sis190_pci_tbl[] __devinitdata = {
> +static struct pci_device_id sis190_pci_tbl[] __devinitconst = {
> { PCI_DEVICE(PCI_VENDOR_ID_SI, 0x0190), 0, 0, 0 },
> { PCI_DEVICE(PCI_VENDOR_ID_SI, 0x0191), 0, 0, 1 },
> { 0, },
sis190_pci_tbl is not const...
> @@ -1556,7 +1556,7 @@ static int __devinit
> sis190_get_mac_addr_from_eeprom(struct pci_dev *pdev,
> static int __devinit sis190_get_mac_addr_from_apc(struct pci_dev *pdev,
> struct net_device *dev)
> {
> - static const u16 __devinitdata ids[] = { 0x0965, 0x0966, 0x0968 };
> + static u16 __devinitconst ids[] = { 0x0965, 0x0966, 0x0968 };
> struct sis190_private *tp = netdev_priv(dev);
> struct pci_dev *isa_bridge;
> u8 reg, tmp8;
> --
> 1.5.3.8
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [EMAIL PROTECTED]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [SIS190] Constify data marked as __devinitdata
On Wed, Jan 30, 2008 at 12:23:23PM +0100, Jan Engelhardt wrote:
>
> On Jan 30 2008 11:53, Jonas Bonn wrote:
> >
> >This fixes build error as gcc complains about a "section type conflict"
> >due to the const __devinitdata in sis190_get_mac_addr_from_apc().
>
> >-static struct pci_device_id sis190_pci_tbl[] __devinitdata = {
> >+static const struct pci_device_id sis190_pci_tbl[] __devinitdata = {
> > { PCI_DEVICE(PCI_VENDOR_ID_SI, 0x0190), 0, 0, 0 },
> > { PCI_DEVICE(PCI_VENDOR_ID_SI, 0x0191), 0, 0, 1 },
> > { 0, },
>
> Eh? Did you mean to
>
> -static const u16 __devinitdata ids[] = { 0x0965, 0x0966, 0x0968 };
> +static u16 __devinitdata ids[] = { 0x0965, 0x0966, 0x0968 };
>
> instead? Because AFAIK, const *and* __sectionmarker does not mix.
We have just introduced __initconst, __cpuinitconst, __meminitconst
for const data.
So the patch is wrong.
Sam
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [git patches] net driver fixes
>
> Jeff Garzik (1):
> [netdrvr] sis190: build fix
But you did it wrong...
sis190.c b/drivers/net/sis190.c
> index b570402..2e9e88b 100644
> --- a/drivers/net/sis190.c
> +++ b/drivers/net/sis190.c
> @@ -326,7 +326,7 @@ static const struct {
> { "SiS 191 PCI Gigabit Ethernet adapter" },
> };
>
> -static struct pci_device_id sis190_pci_tbl[] __devinitdata = {
> +static struct pci_device_id sis190_pci_tbl[] = {
> { PCI_DEVICE(PCI_VENDOR_ID_SI, 0x0190), 0, 0, 0 },
> { PCI_DEVICE(PCI_VENDOR_ID_SI, 0x0191), 0, 0, 1 },
> { 0, },
The __devinitdata is OK, it is the following _devinitdata that had to be
_devinitconst.
Sam
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: sis190 build breakage
On Wed, Jan 30, 2008 at 03:22:51AM -0500, Jeff Garzik wrote:
> Sam Ravnborg wrote:
> >On Tue, Jan 29, 2008 at 11:03:10PM +0100, Francois Romieu wrote:
> >>maximilian attems <[EMAIL PROTECTED]> :
> >>> CC [M] drivers/net/sis190.o
> >>> drivers/net/sis190.c:329: error: sis190_pci_tbl causes a section type
> >>> conflict
> >>> make[5]: *** [drivers/net/sis190.o] Error 1
> >>>
> >>>gcc --version
> >>>gcc (GCC) 4.1.2 20061115 (prerelease) (Debian 4.1.1-21)
> >
> >Looks like a bug where __initdata has been used
> >for const data.
> >Searching:
> >static int __devinit sis190_get_mac_addr_from_apc(struct pci_dev *pdev,
> > struct net_device *dev)
> >{
> >static const u16 __devinitdata ids[] = { 0x0965, 0x0966, 0x0968 };
> >struct sis190_private *tp = netdev_priv(dev);
> >struct pci_dev *isa_bridge;
> >u8 reg, tmp8;
> >
> >Try to change this is __initconst and it should be fixed.
>
> We have __initconst now?
>
> Three cheers, and a beer, to whomever did that...
I will hand over the cheers to Jan Beulich and drink the beer myself ;-)
Sam
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUILD FAILURE]2.6.24-git6 build failure on sis190 ethernet driver
On Wed, Jan 30, 2008 at 09:11:36AM +0530, Kamalesh Babulal wrote:
> Hi,
>
> The 2.6.24-git6 kernel build fails on various x86_64 machines with the build
> failure
>
> drivers/net/sis190.c:329: error: sis190_pci_tbl causes a section type conflict
> make[2]: *** [drivers/net/sis190.o] Error 1
>
> # gcc --version (machine1)
> gcc (GCC) 4.1.1 20070105 (Red Hat 4.1.1-52)
>
> # gcc --version (machine2)
> gcc (GCC) 4.1.1 20060525 (Red Hat 4.1.1-1)
Hi Kamalesh
I know another patch is circulating, but please try the following.
diff --git a/drivers/net/sis190.c b/drivers/net/sis190.c
index b570402..0a5e024 100644
--- a/drivers/net/sis190.c
+++ b/drivers/net/sis190.c
@@ -1556,7 +1556,7 @@ static int __devinit
sis190_get_mac_addr_from_eeprom(struct pci_dev *pdev,
static int __devinit sis190_get_mac_addr_from_apc(struct pci_dev *pdev,
struct net_device *dev)
{
- static const u16 __devinitdata ids[] = { 0x0965, 0x0966, 0x0968 };
+ static const u16 __devinitconst ids[] = { 0x0965, 0x0966, 0x0968 };
struct sis190_private *tp = netdev_priv(dev);
struct pci_dev *isa_bridge;
u8 reg, tmp8;
It is the better fix if you can confirm it working.
The section conflict issued by gcc happens because we try to
mix const and non-const data in the same section.
Sam
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: sis190 build breakage
On Tue, Jan 29, 2008 at 11:03:10PM +0100, Francois Romieu wrote:
> maximilian attems <[EMAIL PROTECTED]> :
> > CC [M] drivers/net/sis190.o
> > drivers/net/sis190.c:329: error: sis190_pci_tbl causes a section type
> > conflict
> > make[5]: *** [drivers/net/sis190.o] Error 1
> >
> > gcc --version
> > gcc (GCC) 4.1.2 20061115 (prerelease) (Debian 4.1.1-21)
Looks like a bug where __initdata has been used
for const data.
Searching:
static int __devinit sis190_get_mac_addr_from_apc(struct pci_dev *pdev,
struct net_device *dev)
{
static const u16 __devinitdata ids[] = { 0x0965, 0x0966, 0x0968 };
struct sis190_private *tp = netdev_priv(dev);
struct pci_dev *isa_bridge;
u8 reg, tmp8;
Try to change this is __initconst and it should be fixed.
Sam
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [2.6 patch] atm/suni.c: fix section mismatch
On Sat, Jan 19, 2008 at 03:18:51PM +0200, Adrian Bunk wrote:
> EXPORT_SYMBOL'ed code mustn't be __*init.
>
> Signed-off-by: Adrian Bunk <[EMAIL PROTECTED]>
Acked-by: Sam Ravnborg <[EMAIL PROTECTED]>
>
> ---
> --- linux-2.6/drivers/atm/suni.c.old 2008-01-19 12:16:41.0 +0200
> +++ linux-2.6/drivers/atm/suni.c 2008-01-19 12:17:23.0 +0200
> @@ -275,35 +275,35 @@ static int suni_stop(struct atm_dev *dev
> *walk = PRIV((*walk)->dev)->next;
> if (!sunis) del_timer_sync(&poll_timer);
> spin_unlock_irqrestore(&sunis_lock,flags);
> kfree(PRIV(dev));
>
> return 0;
> }
>
>
> static const struct atmphy_ops suni_ops = {
> .start = suni_start,
> .ioctl = suni_ioctl,
> .interrupt = suni_int,
> .stop = suni_stop,
> };
>
>
> -int __devinit suni_init(struct atm_dev *dev)
> +int suni_init(struct atm_dev *dev)
> {
> unsigned char mri;
>
> mri = GET(MRI); /* reset SUNI */
> PUT(mri | SUNI_MRI_RESET,MRI);
> PUT(mri,MRI);
> PUT((GET(MT) & SUNI_MT_DS27_53),MT); /* disable all tests */
> REG_CHANGE(SUNI_TPOP_APM_S,SUNI_TPOP_APM_S_SHIFT,SUNI_TPOP_S_SONET,
> TPOP_APM); /* use SONET */
> REG_CHANGE(SUNI_TACP_IUCHP_CLP,0,SUNI_TACP_IUCHP_CLP,
> TACP_IUCHP); /* idle cells */
> PUT(SUNI_IDLE_PATTERN,TACP_IUCPOP);
> dev->phy = &suni_ops;
> return 0;
> }
>
> EXPORT_SYMBOL(suni_init);
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to [EMAIL PROTECTED]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [2.6 patch] atm/idt77105.c: fix section mismatch
On Sat, Jan 19, 2008 at 03:18:49PM +0200, Adrian Bunk wrote:
> EXPORT_SYMBOL'ed code mustn't be __*init.
>
> Signed-off-by: Adrian Bunk <[EMAIL PROTECTED]>
Acked-by: Sam Ravnborg <[EMAIL PROTECTED]>
>
> ---
> --- linux-2.6/drivers/atm/idt77105.c.old 2008-01-19 11:19:53.0
> +0200
> +++ linux-2.6/drivers/atm/idt77105.c 2008-01-19 11:20:10.0 +0200
> @@ -354,13 +354,13 @@ static const struct atmphy_ops idt77105_
> .ioctl =idt77105_ioctl,
> .interrupt =idt77105_int,
> .stop = idt77105_stop,
> };
>
>
> -int __devinit idt77105_init(struct atm_dev *dev)
> +int idt77105_init(struct atm_dev *dev)
> {
> dev->phy = &idt77105_ops;
> return 0;
> }
>
> EXPORT_SYMBOL(idt77105_init);
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to [EMAIL PROTECTED]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] veth: move veth.h to include/linux
On Tue, Dec 25, 2007 at 12:46:56PM -0800, Stephen Hemminger wrote:
> Move veth.h from net/ to linux/ since it is a user api, and
> add it to user header processing Kbuild.
>
> Signed-off-by: Stephen Hemminger <[EMAIL PROTECTED]>
>
> --- a/drivers/net/veth.c 2007-12-25 12:30:56.0 -0800
> +++ b/drivers/net/veth.c 2007-12-25 12:31:38.0 -0800
> @@ -15,7 +15,7 @@
>
> #include
> #include
> -#include
> +#include
>
> #define DRV_NAME "veth"
> #define DRV_VERSION "1.0"
> --- /dev/null 1970-01-01 00:00:00.0 +
> +++ b/include/linux/veth.h2007-10-16 16:48:20.0 -0700
> @@ -0,0 +1,12 @@
> +#ifndef __NET_VETH_H_
> +#define __NET_VETH_H_
> +
> +enum {
> + VETH_INFO_UNSPEC,
> + VETH_INFO_PEER,
> +
> + __VETH_INFO_MAX
> +#define VETH_INFO_MAX(__VETH_INFO_MAX - 1)
> +};
> +
> +#endif
> --- a/include/net/veth.h 2007-12-25 12:33:49.0 -0800
> +++ /dev/null 1970-01-01 00:00:00.0 +
> @@ -1,12 +0,0 @@
> -#ifndef __NET_VETH_H_
> -#define __NET_VETH_H_
> -
> -enum {
> - VETH_INFO_UNSPEC,
> - VETH_INFO_PEER,
> -
> - __VETH_INFO_MAX
> -#define VETH_INFO_MAX(__VETH_INFO_MAX - 1)
> -};
> -
> -#endif
> --- a/include/linux/Kbuild2007-12-25 12:44:59.0 -0800
> +++ b/include/linux/Kbuild2007-12-25 12:45:31.0 -0800
> @@ -342,6 +342,7 @@ unifdef-y += unistd.h
> unifdef-y += usbdevice_fs.h
> unifdef-y += user.h
> unifdef-y += utsname.h
> +unifdef-y += veth.h
> unifdef-y += videodev2.h
> unifdef-y += videodev.h
> unifdef-y += virtio_config.h
Someone will argue that you should use header-y +=
because the file has no conditionals removed by unifdef.
My personal opinion is that we should kill header-y and
I had patches to greatly improve all this but they got
lost by accident and I have not yet redone them.
Sam
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to [EMAIL PROTECTED]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: A short question about net git tree and patches
On Thu, Dec 20, 2007 at 03:22:58AM -0800, David Miller wrote: > From: Sam Ravnborg <[EMAIL PROTECTED]> > Date: Thu, 20 Dec 2007 10:55:10 +0100 > > > net-2.6.25.git <= patches for current kernel release (only fixes) > > net-2.6.git <= patches for next kernel relase and planned to be > > applied in next merge window > > > > So net-2.6.git is the correct choice for bleeding edge. > > You reversed them, net-2.6.25.git is for bleeding edge > stuff, net-2.6.git is for bug fixes only. Sorry - thanks for clarifying it. Sam - who should refrain from thinking too much in current crappy condition -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: A short question about net git tree and patches
On Thu, Dec 20, 2007 at 11:20:26AM +0200, David Shwatrz wrote: > Hello, > I have a short question regarding the net git tree and patches: > I want to write and send patches against the most recent and > bleeding edge kernel networking code. > I see in: > http://kernel.org/pub/scm/linux/kernel/git/davem/?C=M;O=A > that there are 3 git trees which can be candidates for git-clone and > making patches against; > these are: > netdev-2.6.git, net-2.6.25.git and net-2.6.git. > > It seems to me that net-2.6.git is the most suitable one to work against; > am I right ? > what is the difference, in short, between the three repositories? IIRC the usage is: netdev-2.6.git <= old stuff, 4 weeks since last update. Not in use net-2.6.25.git <= patches for current kernel release (only fixes) net-2.6.git <= patches for next kernel relase and planned to be applied in next merge window So net-2.6.git is the correct choice for bleeding edge. Sam -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] [5/9] modpost: Fix a buffer overflow in modpost
On Mon, Dec 10, 2007 at 08:57:28PM +0100, Andi Kleen wrote: > On Monday 10 December 2007 20:32, Sam Ravnborg wrote: > > On Thu, Nov 22, 2007 at 03:43:10AM +0100, Andi Kleen wrote: > > > When passing an file name > 1k the stack could be overflowed. > > > Not really a security issue, but still better plugged. > > > > Looks good. A s-o-b line again please. > > Signed-off-by: Andi Kleen <[EMAIL PROTECTED]> > > > Although I am not so happy with the ue of gcc extensions. > > That's not a gcc extension. It's C99. OK. I have applied all three patches to kbuild.git. As I did not follow the whole thread about the namespace I did not take those. And the first patch touching module.c should go in via akpm I think. It is outside my core-competence area at least . Sam -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] [5/9] modpost: Fix a buffer overflow in modpost
On Thu, Nov 22, 2007 at 03:43:10AM +0100, Andi Kleen wrote:
>
> When passing an file name > 1k the stack could be overflowed.
> Not really a security issue, but still better plugged.
Looks good. A s-o-b line again please.
Although I am not so happy with the ue of gcc extensions.
Sam
>
>
> ---
> scripts/mod/modpost.c |3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> Index: linux/scripts/mod/modpost.c
> ===
> --- linux.orig/scripts/mod/modpost.c
> +++ linux/scripts/mod/modpost.c
> @@ -1656,7 +1656,6 @@ int main(int argc, char **argv)
> {
> struct module *mod;
> struct buffer buf = { };
> - char fname[SZ];
> char *kernel_read = NULL, *module_read = NULL;
> char *dump_write = NULL;
> int opt;
> @@ -1709,6 +1708,8 @@ int main(int argc, char **argv)
> err = 0;
>
> for (mod = modules; mod; mod = mod->next) {
> + char fname[strlen(mod->name) + 10];
> +
> if (mod->skip)
> continue;
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] [4/9] modpost: Fix format string warnings
On Thu, Nov 22, 2007 at 03:43:09AM +0100, Andi Kleen wrote:
>
> Fix wrong format strings in modpost exposed by the previous patch.
> Including one missing argument -- some random data was printed instead.
Looks good. Can I get a s-o-b then I will apply it.
Sam
>
> ---
> scripts/mod/modpost.c |7 ---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> Index: linux/scripts/mod/modpost.c
> ===
> --- linux.orig/scripts/mod/modpost.c
> +++ linux/scripts/mod/modpost.c
> @@ -388,7 +388,7 @@ static int parse_elf(struct elf_info *in
>
> /* Check if file offset is correct */
> if (hdr->e_shoff > info->size) {
> - fatal("section header offset=%u in file '%s' is bigger then
> filesize=%lu\n", hdr->e_shoff, filename, info->size);
> + fatal("section header offset=%lu in file '%s' is bigger then
> filesize=%lu\n", (unsigned long)hdr->e_shoff, filename, info->size);
> return 0;
> }
>
> @@ -409,7 +409,7 @@ static int parse_elf(struct elf_info *in
> const char *secname;
>
> if (sechdrs[i].sh_offset > info->size) {
> - fatal("%s is truncated. sechdrs[i].sh_offset=%u >
> sizeof(*hrd)=%ul\n", filename, (unsigned int)sechdrs[i].sh_offset,
> sizeof(*hdr));
> + fatal("%s is truncated. sechdrs[i].sh_offset=%lu >
> sizeof(*hrd)=%lu\n", filename, (unsigned long)sechdrs[i].sh_offset,
> sizeof(*hdr));
> return 0;
> }
> secname = secstrings + sechdrs[i].sh_name;
> @@ -907,7 +907,8 @@ static void warn_sec_mismatch(const char
>"before '%s' (at offset -0x%llx)\n",
>modname, fromsec, (unsigned long long)r.r_offset,
>secname, refsymname,
> - elf->strtab + after->st_name);
> + elf->strtab + after->st_name,
> + (unsigned long long)r.r_offset);
> } else {
> warn("%s(%s+0x%llx): Section mismatch: reference to %s:%s\n",
>modname, fromsec, (unsigned long long)r.r_offset,
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] [3/9] modpost: Declare the modpost error functions as printf like
On Thu, Nov 22, 2007 at 03:43:08AM +0100, Andi Kleen wrote:
>
> This way gcc can warn for wrong format strings
This loks good. Can I get i s-o-b then I will apply it.
Sam
>
> ---
> scripts/mod/modpost.c |8 +---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> Index: linux/scripts/mod/modpost.c
> ===
> --- linux.orig/scripts/mod/modpost.c
> +++ linux/scripts/mod/modpost.c
> @@ -33,7 +33,9 @@ enum export {
> export_unused_gpl, export_gpl_future, export_unknown
> };
>
> -void fatal(const char *fmt, ...)
> +#define PRINTF __attribute__ ((format (printf, 1, 2)))
> +
> +PRINTF void fatal(const char *fmt, ...)
> {
> va_list arglist;
>
> @@ -46,7 +48,7 @@ void fatal(const char *fmt, ...)
> exit(1);
> }
>
> -void warn(const char *fmt, ...)
> +PRINTF void warn(const char *fmt, ...)
> {
> va_list arglist;
>
> @@ -57,7 +59,7 @@ void warn(const char *fmt, ...)
> va_end(arglist);
> }
>
> -void merror(const char *fmt, ...)
> +PRINTF void merror(const char *fmt, ...)
> {
> va_list arglist;
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] move unneeded data to initdata section
On Thu, Nov 15, 2007 at 10:17:14PM +0300, Denis V. Lunev wrote: > > will you mind against this? > diff --git a/include/net/net_namespace.h b/include/net/net_namespace.h > index 5dd6d90..d136707 100644 > --- a/include/net/net_namespace.h > +++ b/include/net/net_namespace.h > @@ -119,10 +119,14 @@ static inline struct net *maybe_get_net(struct net *net) > #ifdef CONFIG_NET_NS > #define __net_init > #define __net_exit > -#define __net_initdata > #else > #define __net_init __init > #define __net_exit __exit_refok > +#endif > + > +#if defined(CONFIG_NET_NS) || defined(MODULE) > +#define __net_initdata > +#else > #define __net_initdata __initdata > #endif n principle I am against this approach. __initdata is far too overloaded with different stuff. A much more preferred approach should be to create new sections named for example .init.data.net and .init.data.net.module And then in include/asm-generic/vmlinux.lds.h decide the location of these sections. On top of this we would have to teach modpost about these new sections. But the advantage of this approach is that the section mismatch checks are *independent* of the module being a MODULE or build-in. The check will still happen. In this way we avoid the situation where a warning only pops up in certain configurations. To do so will obviously require a bit more linker script consolidation but if you or some else could step in a do this it would be great! Sam - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] move unneeded data to initdata section
On Thu, Nov 15, 2007 at 11:19:26AM -0700, Eric W. Biederman wrote: > Sam Ravnborg <[EMAIL PROTECTED]> writes: > > > On Thu, Nov 15, 2007 at 05:42:04PM +0300, Denis V. Lunev wrote: > >> > >> nothing is discarded after module load. Though, I can be wrong. Could > >> you point me to the exact place? > > If __initdata is not discarded after module load then we should do it. > > There is no reason to waste __initdata RAM when the module is loaded. > > Down at the bottom of sys_init_module we have: > > /* Drop initial reference. */ > module_put(mod); > unwind_remove_table(mod->unwind_info, 1); > > module_free(mod, mod->module_init); > ^ > mod->module_init = NULL; > mod->init_size = 0; > mod->init_text_size = 0; > mutex_unlock(&module_mutex); > > return 0; > > Which frees the memory for the .init sections. Thanks for clarifying this Eric - should have looked myself.. Sam - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] move unneeded data to initdata section
On Thu, Nov 15, 2007 at 05:42:04PM +0300, Denis V. Lunev wrote: > Eric W. Biederman wrote: > > "Denis V. Lunev" <[EMAIL PROTECTED]> writes: > > > >> This patch reverts Eric's commit 2b008b0a8e96b726c603c5e1a5a7a509b5f61e35 > >> > >> It diets .text & .data section of the kernel if CONFIG_NET_NS is not set. > >> This is safe after list operations cleanup. > > > > Ok. This patch is technically safe because none of the touched > > code can live in a module and so we never touch the exit code path. > > > > However in the general case and as a code idiom this __net_initdata > > on struct pernet_operations is fundamentally horribly broken. > > > > Look at what happens if we use this idiom in module. There > > is only one definition of __initdata ".init.data". The module > > loader places all sections that begin with .init in a region of > > memory that will be discarded after module initialization. > > nothing is discarded after module load. Though, I can be wrong. Could > you point me to the exact place? If __initdata is not discarded after module load then we should do it. There is no reason to waste __initdata RAM when the module is loaded. Sam - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/7] CAN: Add PF_CAN core module
On Thu, Nov 15, 2007 at 04:05:30AM -0800, David Miller wrote:
> From: Urs Thuermann <[EMAIL PROTECTED]>
> Date: 15 Nov 2007 12:51:34 +0100
>
> > I prefer our code because it is shorter (fits into one line) and can
> > be used anywhere where an expression is allowed compared to only where
> > a statement is allowed. Actually, I first had
> >
> > #define DBG( ... ) ((debug & 1) && printk( ... ))
> >
> > and so on, but that didn't work with can_debug_{cframe,sbk} since they
> > return void.
> >
> > Admitted, the benefit of expr vs. statement is really negligible and
> > since this issue has come up several times I will change these macros
> > using do-while.
>
> I really frown upon these local debugging macros people tend to want
> to submit with their changes.
>
> It really craps up the tree, even though it might be useful to you.
>
> So please remove this stuff or replace the debugging statements
> with some generic kernel debugging facility, there are several.
It would be usefull if someone could make a short intro to the preferred
ones and we could stuff it in Documentation/*
Had same comment but had nowhere to point the can guys at.
Sam
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/7] CAN: Add PF_CAN core module
> > > + > > > +struct timer_list stattimer; /* timer for statistics update */ > > > +struct s_stats stats; /* packet statistics */ > > > +struct s_pstats pstats; /* receive list statistics */ > > > > More global variables without prefix. > > These variables are not exported with EXPORT_SYMBOL, so there should > be no name conflict. They cannot be made static because they are used > in af_can.c and proc.c. Nevertheless we can prefix them with can_ if > you still think it's necessary. When this is build-in they will be in the global kernel namespace. So please add can_ prefix. Sam - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG] New Kernel Bugs
On Wed, Nov 14, 2007 at 06:56:06AM +0100, Sam Ravnborg wrote: > > > > > If so, MANITAINERS claims that it is subscribers-only. That would cause > > > some bug reporters to give up and go away. > > > > Find some other mailing list; I'm not hosting *nor* am I willing to run a > > non-subscribers only mailing list. Period. Not negotiable, so don't even > > try to change my mind. > > The postmasters at vger is pretty good at running mailing lists. > For linux-kbuild my effort so far has been to request it. > Thats not a big deal. > > So if they accept it you could have [EMAIL PROTECTED] for zero > overhead for you. And in a later mail I saw davem already created it. Sam - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG] New Kernel Bugs
> > > If so, MANITAINERS claims that it is subscribers-only. That would cause > > some bug reporters to give up and go away. > > Find some other mailing list; I'm not hosting *nor* am I willing to run a > non-subscribers only mailing list. Period. Not negotiable, so don't even > try to change my mind. The postmasters at vger is pretty good at running mailing lists. For linux-kbuild my effort so far has been to request it. Thats not a big deal. So if they accept it you could have [EMAIL PROTECTED] for zero overhead for you. Sam - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] proc_fs.h redux
> > As a general rule, I think it better to use includes > than use naked forward declarations. Quite the opposite - at least in the kernel source. The general rule is that a .h file shall include the .h files which contain declarations used by said .h files. But naked declarations as above is preferred over including a full header file. We see the full header dependency thing to blow off when inline function are used - which is more and more the case. In several cases we have converted inline functions to macros just to simplify the nightmare of header dependencies we have. Arnaldo have a nice script that generate a .ps file showing all the dependencies. He lately posted this URL: http://oops.ghostprotocols.net:81/acme/tcp.h.ps Sam - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: bnx2 dirver's firmware images
Hi Maks. On Wed, Sep 19, 2007 at 07:18:41PM +0200, maximilian attems wrote: > On Wed, Sep 19, 2007 at 10:12:38AM -0700, H. Peter Anvin wrote: > > > > Could you give a concrete example? > > > > -hpa > > [EMAIL PROTECTED]:~/src/klibc$ touch usr/utils/mount_main.c > [EMAIL PROTECTED]:~/src/klibc$ make > KLIBCCC usr/utils/mount_main.o > KLIBCLD usr/utils/static/halt > LN usr/utils/static/reboot > LN usr/utils/static/poweroff > KLIBCLD usr/utils/shared/halt > LN usr/utils/shared/reboot > LN usr/utils/shared/poweroff > KLIBCLD usr/utils/static/chroot > KLIBCLD usr/utils/static/dd > I do not recall the details at the moment but I recall I had to do this tradeoff for some reasons. I do not say it is not fixable but when I did the static-y support I recall I took a shortcut or two under the assumption that everything build by klibc was anyway so simple that compiling a bit too much was no harm. Too buried with other stuff right now. But feel free to poke me in roughly a month and I will take a deeper look. Sam - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: bnx2 dirver's firmware images
On Tue, Sep 18, 2007 at 01:08:10PM -0700, David Miller wrote: > From: "H. Peter Anvin" <[EMAIL PROTECTED]> > Date: Tue, 18 Sep 2007 12:27:04 -0700 > > > Well, what I was referring to here, of course, was the initramfs > > integrated in the kernel image, so it all comes out of the kernel build > > tree and produces a single bootable image. The fact that part of it > > contains userspace code is in that way invisible. > > > > That was kind of the point here, and the only reason for pushing klibc > > into the kernel build tree at all. Under the "distros use external > > initrd anyway" school of thought, whatever libc used for that can be > > external anyway. > > Sounds good to me :) Except there seems to be great resistance to include userland code in the kernel as demonstrated at last KS. Or this is maybe just a single vocal person and the topic were brought up late? Anyway - if we again consider klibc I will do my best to make the build stuff as smooth as possible. Sam - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] jumbo all-NICs ethtool count cleanup
Hi Jeff. You wrote: > The hooks ->self_test_count() and ->get_stats_count() are now unused > in the main tree. So I'm suprised to see more lines added than deleted: > 35 files changed, 346 insertions(+), 246 deletions(-) Puzzled - may need a bit more coffee (morning here).. Sam - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 2.6.23-rc4-mm1 "no CRC" MODPOST warnings
On Sun, Sep 02, 2007 at 03:36:08AM +0530, Satyam Sharma wrote: > > > On Fri, 31 Aug 2007, Andrew Morton wrote: > > > > ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.23-rc4/2.6.23-rc4-mm1/ > > Got these on an i386 build with CONFIG_MODVERSIONS=y ... > > WARNING: "div64_64" [net/netfilter/xt_connbytes.ko] has no CRC! > WARNING: "div64_64" [net/ipv4/tcp_cubic.ko] has no CRC! As Adrian already commented it is fixed in kbuild.git. It happes bacause genksyms did not know __extension__ and error recovery in the parser were bad. I only managed to add support for __extension__ but the error receovery are not fixed :-( kbuild.git is not part of this -mm due to me fucking up the above fix. That is corrected now so it will be in next -mm. Sam - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] net/, drivers/net/ , missing EXPERIMENTAL in menus
On Sat, Sep 01, 2007 at 06:44:06AM -0400, Robert P. J. Day wrote:
>
> > > while (1) {
> > > printf("%*s%s ", indent - 1, "", menu->prompt->text);
> > > + switch (sym->maturity) {
> > > + case M_EXPERIMENTAL:
> > > + printf("(EXPERIMENTAL) ");
> > > + break;
> > > + case M_DEPRECATED:
> > > + printf("(DEPRECATED) ");
> > > + break;
> > > + case M_OBSOLETE:
> > > + printf("(OBSOLETE) ");
> > > + break;
> > > + case M_BROKEN:
> > > + printf("(BROKEN) ");
> > > + break;
> > > + default:
> > > + break;
> > > + }
> > > if (sym->name)
> > > printf("(%s) ", sym->name);
> > > type = sym_get_type(sym);
>
> for now, simon, why not just reduce this to supporting only DEPRECATED
> and OBSOLETE so that it can be at least tested as "proof of concept?"
The principle with letting a dependency add text to the promts are good.
But the implementation done by Simon with a language extension is not good.
A simple and better approach would be to use the newly added option
support for this and let the backend generate the promtps.
I have not yet tried to cook up a patch for it, but it should be
quite generaic and doable.
config EXPERIMENTAL
option appendprompt=" (EXPERIMENTAL)
config DEPRECATED
option appendprompt=" (DEPRECATED)
Then the dependency added will automatically append to the prompt.
It would probarly have be done in two steps. One that introduce a helper
function to retreive the prompt text and introduce it. And next a patch to
add a text if a symbol has a dependency on a symbol with a specific option
assigned.
Sam
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [-mm patch] make types.h usable for non-gcc C parsers
On Tue, Aug 28, 2007 at 07:59:04PM +0200, Adrian Bunk wrote:
> On Tue, Aug 28, 2007 at 07:06:04PM +0200, Sam Ravnborg wrote:
> > >
> > > It fixes a bug exposed by a -mm only patch, not by the net tree
> > > (and 2.6.23-rc3-mm1 doesn't contain the net tree at all).
> > >
> > > > But I'd like a better description, please. Which "non-gcc parser" are
> > > > we
> > > > talking about here? Something under ./scripts/. Well, please identify
> > > > it,
> > > > and describe what the problem is, and how the proposed patch will
> > > > address
> > > > it.
> > > >...
> > >
> > > It's about parsers like the Sun C compiler and the C parser shipped
> > > with genksyms.
> >
> > So it is about two bugs.
> > 1) kbuild (genksyms) fails to generate CRC for some symbols
> > 2) allow userspace to parse the header
> >
> > As for 2 we already use sed to remove a lot of stuff in our headers
> > so why do we use another approach here?
>
> This time it's the other way round:
>
> We need __extension__ only in userspace.
>
> > As for 1 I will try to teach genksyms to accept __extension__ but
> > it seems leess trivial than I expected (most be fooling myself somehow).
>
> We anyway need a way to hide __extension__ from non-gcc userspace C
> compilers, and it can be hidden from genksyms the same way.
OK.
I have anyway added support for __extension__ in genksyms.
See below patch.
Note: To try this patch out do the following in a fresh tree (no generated
files):
$ rm scripts/genksyms/*_shipped
$ apply patch
$ make GENERATE_PARSER=1 ...
In kbuild.git the _shipped files are updated but that would just be noise here.
Sam
>From 26132bc829651acce2f124f78dfea43120e52c31 Mon Sep 17 00:00:00 2001
From: Sam Ravnborg <[EMAIL PROTECTED]>
Date: Tue, 28 Aug 2007 20:28:55 +0200
Subject: [PATCH] kbuild: __extension__ support in genksyms (fix unknown CRC
warning)
Recently the __extension__ keyword has been introduced in the kernel.
Teach genksyms about this keyword so it can generate correct CRC for
exported symbols that uses a symbol marked __extension__.
For now only the typedef variant:
__extension__ typedef ...
is supported.
Later we may add more variants as needed.
This patch contains the actual source file changes. The
following patch will hold modifications to the generated
files (*_shipped) and only after the second patch the fix
has effect.
Signed-off-by: Sam Ravnborg <[EMAIL PROTECTED]>
---
scripts/genksyms/keywords.gperf |1 +
scripts/genksyms/parse.y|5 -
2 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/scripts/genksyms/keywords.gperf b/scripts/genksyms/keywords.gperf
index c75e0c8..5ef3733 100644
--- a/scripts/genksyms/keywords.gperf
+++ b/scripts/genksyms/keywords.gperf
@@ -11,6 +11,7 @@ __attribute, ATTRIBUTE_KEYW
__attribute__, ATTRIBUTE_KEYW
__const, CONST_KEYW
__const__, CONST_KEYW
+__extension__, EXTENSION_KEYW
__inline, INLINE_KEYW
__inline__, INLINE_KEYW
__signed, SIGNED_KEYW
diff --git a/scripts/genksyms/parse.y b/scripts/genksyms/parse.y
index ca04c94..408cdf8 100644
--- a/scripts/genksyms/parse.y
+++ b/scripts/genksyms/parse.y
@@ -61,6 +61,7 @@ remove_list(struct string_list **pb, struct string_list **pe)
%token DOUBLE_KEYW
%token ENUM_KEYW
%token EXTERN_KEYW
+%token EXTENSION_KEYW
%token FLOAT_KEYW
%token INLINE_KEYW
%token INT_KEYW
@@ -110,7 +111,9 @@ declaration:
;
declaration1:
- TYPEDEF_KEYW { is_typedef = 1; } simple_declaration
+ EXTENSION_KEYW TYPEDEF_KEYW { is_typedef = 1; } simple_declaration
+ { $$ = $4; }
+ | TYPEDEF_KEYW { is_typedef = 1; } simple_declaration
{ $$ = $3; }
| simple_declaration
| function_definition
--
1.5.1.rc3.2928.g8e573-dirty
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [-mm patch] make types.h usable for non-gcc C parsers
> > It fixes a bug exposed by a -mm only patch, not by the net tree > (and 2.6.23-rc3-mm1 doesn't contain the net tree at all). > > > But I'd like a better description, please. Which "non-gcc parser" are we > > talking about here? Something under ./scripts/. Well, please identify it, > > and describe what the problem is, and how the proposed patch will address > > it. > >... > > It's about parsers like the Sun C compiler and the C parser shipped > with genksyms. So it is about two bugs. 1) kbuild (genksyms) fails to generate CRC for some symbols 2) allow userspace to parse the header As for 2 we already use sed to remove a lot of stuff in our headers so why do we use another approach here? As for 1 I will try to teach genksyms to accept __extension__ but it seems leess trivial than I expected (most be fooling myself somehow). Sam - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [-mm patch] make types.h usable for non-gcc C parsers
On Tue, Aug 28, 2007 at 12:37:04AM -0700, Andrew Morton wrote: > On Mon, 27 Aug 2007 23:27:43 +0200 Adrian Bunk <[EMAIL PROTECTED]> wrote: > > > On Wed, Aug 22, 2007 at 03:33:27PM +0200, Gabriel C wrote: > > >... > > > WARNING: "div64_64" [net/netfilter/xt_connbytes.ko] has no CRC! > > >... > > > > Patch below. > > > > > Regards, > > > > > > Gabriel > > > > cu > > Adrian > > > > > > <-- snip --> > > > > > > This patch makes the 64bit integers on 32bit architectures usable for > > all C parsers that know about "long long". > > > > Signed-off-by: Adrian Bunk <[EMAIL PROTECTED]> > > > > Given that this patch (hopefully) fixes a problem in the current net-2.6.24 > tree, I'm inclined to slip it into mainline immediately. > > But I'd like a better description, please. Which "non-gcc parser" are we > talking about here? Something under ./scripts/. Well, please identify it, > and describe what the problem is, and how the proposed patch will address > it. > > Let's cc Sam too, as I guess he's the guy whose code just broke. If my analysis is correct then genksyms fails to produce a CRC for div64_64 because genksyms does not know the __extension__ keyword. And this patch just paper over the real bug wich is in genksyms - right? So we should fix the root cause here. Googeling I did not find a good description of where __extension__ can be used so I fail to see where in the parse.y file I shal add the keyword. I think __extension__ may be used both as a part of an expression AND as part of a typedef (as in this case) but I wonder if this is where it is limited to be used. I would like to have this sorted out so we do not do a half-backed solution, and the proposed patch as it just paper over the real bug is no good. Sam - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: NETPOLL=y , NETDEVICES=n compile error ( Re: 2.6.23-rc1-mm1 )
> > > > ... > > endif # NETDEVICES > > > > config NETPOLL > > depends on NETDEVICES > > def_bool NETCONSOLE > > > > config NETPOLL_TRAP > > bool "Netpoll traffic trapping" > > default n > > depends on NETPOLL > > > > config NET_POLL_CONTROLLER > > def_bool NETPOLL > > depends on NETPOLL > > > > > > seems to select NET_POLL_CONTROLLER after selecting NETPOLL, but > > still doesn't check for NETDEVICES dependency. > > That's odd. Adding Sam to the cc:. select is evil select will by brute force set a symbol equal to 'y' without visiting the dependencies. So abusing select you are able to select a symbol FOO even if FOO depends on BAR that is not set. In general use select only for non-visible symbols (no promts anywhere) and for symbols with no dependencies. That will limit the suefullness but on the other hand avoid the illegal configurations all over. kconfig should one day warn about such things but I have not fel inclined to dive into the matters hoping that Roman does one day. Sam - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: modpost warning question
On Wed, Jul 25, 2007 at 06:08:03PM +0800, chengong wrote: > On Wed, 2007-07-25 at 09:27 +0200, Sam Ravnborg wrote: > > On Wed, Jul 25, 2007 at 02:14:12AM -0500, Kumar Gala wrote: > > > I'm seeing the following warning: > > > > > > WARNING: vmlinux.o(.init.text+0x1acdc): Section mismatch: reference to > > > .exit.text:gfar_mdio_exit (between 'gfar_init' and 'gfar_mdio_init') > > > > > > I don't understand why its not ok to access .exit.text from .init.text > > > > Several architectures discards .exit.text in the final linker > > script (arch/$(ARCH)/kernel/vmlinux.lds.S > > > > So any references to .exit.text will when a module is build-in result > > in a linker error because ld will flag it as an error when we reference > > a symbol in a discarded section. > But why? Just make kernel size smaller? Yes - that the whole goal of init/exit sections. > > > > For the popular architectures (i386,x86_64) we discard .exit.text at > > runtime so here we do not see the error from ld (sadly). > >From which version? On my machine I have seen the same problem when > building i386 target with the version 2.6.21. modpost has started to warn about it. I assume you did not see link errors. Sam - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: modpost warning question
> >
> >For the popular architectures (i386,x86_64) we discard .exit.text at
> >runtime so here we do not see the error from ld (sadly).
>
> Fair point, wondering what we do with .exit on PPC, another thing for
> the list :)
from:
arch/ppc/kernel/vmlinux.lds.S:
/* .exit.text is discarded at runtime, not link time,
to deal with references from __bug_table */
.exit.text : { *(.exit.text) }
Sam
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: modpost warning question
On Wed, Jul 25, 2007 at 02:14:12AM -0500, Kumar Gala wrote: > I'm seeing the following warning: > > WARNING: vmlinux.o(.init.text+0x1acdc): Section mismatch: reference to > .exit.text:gfar_mdio_exit (between 'gfar_init' and 'gfar_mdio_init') > > I don't understand why its not ok to access .exit.text from .init.text Several architectures discards .exit.text in the final linker script (arch/$(ARCH)/kernel/vmlinux.lds.S So any references to .exit.text will when a module is build-in result in a linker error because ld will flag it as an error when we reference a symbol in a discarded section. For the popular architectures (i386,x86_64) we discard .exit.text at runtime so here we do not see the error from ld (sadly). Sam - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Netfilter Kconfig: Expose IPv4/6 connection tracking options by selecting NF_CONNTRACK
On Tue, Jul 24, 2007 at 08:36:33AM +0300, Al Boldi wrote: > > Replaces NF_CONNTRACK_ENABLED with NF_CONNTRACK and selects it for > NF_CONNTRACK_IPV4 and NF_CONNTRACK_IPV6 > > This exposes IPv4/6 connection tracking options for easier Kconfig setup. > > Signed-off-by: Al Boldi <[EMAIL PROTECTED]> > Cc: David Miller <[EMAIL PROTECTED]> > Cc: Sam Ravnborg <[EMAIL PROTECTED]> > Cc: Andrew Morton <[EMAIL PROTECTED]> > --- > --- a/net/netfilter/Kconfig 2007-07-09 06:38:52.0 +0300 > +++ b/net/netfilter/Kconfig 2007-07-24 08:28:06.0 +0300 > @@ -25,8 +25,7 @@ config NETFILTER_NETLINK_LOG > and is also scheduled to replace the old syslog-based ipt_LOG > and ip6t_LOG modules. > > -# Rename this to NF_CONNTRACK in a 2.6.25 > -config NF_CONNTRACK_ENABLED > +config NF_CONNTRACK > tristate "Netfilter connection tracking support" > help > Connection tracking keeps a record of what packets have passed > @@ -40,10 +39,6 @@ config NF_CONNTRACK_ENABLED > > To compile it as a module, choose M here. If unsure, say N. > > -config NF_CONNTRACK > - tristate > - default NF_CONNTRACK_ENABLED > - > config NF_CT_ACCT > bool "Connection tracking flow accounting" > depends on NF_CONNTRACK > --- a/net/ipv4/netfilter/Kconfig 2007-07-09 06:38:50.0 +0300 > +++ b/net/ipv4/netfilter/Kconfig 2007-07-24 08:27:39.0 +0300 > @@ -7,7 +7,7 @@ menu "IP: Netfilter Configuration" > > config NF_CONNTRACK_IPV4 > tristate "IPv4 connection tracking support (required for NAT)" > - depends on NF_CONNTRACK > + select NF_CONNTRACK > ---help--- > Connection tracking keeps a record of what packets have passed > through your machine, in order to figure out how they are related > --- a/net/ipv6/netfilter/Kconfig 2007-07-09 06:38:51.0 +0300 > +++ b/net/ipv6/netfilter/Kconfig 2007-07-24 08:27:54.0 +0300 > @@ -7,7 +7,8 @@ menu "IPv6: Netfilter Configuration (EXP > > config NF_CONNTRACK_IPV6 > tristate "IPv6 connection tracking support (EXPERIMENTAL)" > - depends on INET && IPV6 && EXPERIMENTAL && NF_CONNTRACK > + depends on INET && IPV6 && EXPERIMENTAL > + select NF_CONNTRACK > ---help--- > Connection tracking keeps a record of what packets have passed > through your machine, in order to figure out how they are related > This change looks wrong. Due to the reverse nature of "select" kconfig cannot fulfill the dependencies of selected symbols. So as a rule of thumb select should only select symbols with no menu and no dependencies to avoid some of the problems that have popped up during the last months. Sam - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 2.6.23-rc1: BUG_ON in kmap_atomic_prot()
> > For example, missing from the call graph is > > get_page_from_freelist -> > buffered_rmqueue -> [ missing - inlined ] > prep_new_page ->[ missing - inlined ] > prep_zero_page -> [ missing - inlined ] > clear_highpage -> [ missing - inlined ] > kmap_atomic ->[ missing - tailcall ] > kmap_atomic_prot > > (and I'm also pretty sure gcc > is overly aggressive at inlining, and that it causes us pain for > debugging, but whatever) mm/page_alloc.c:static inline void prep_zero_page(struct page *page, int order, gfp_t gfp_flags) include/linux/highmem.h:static inline void clear_highpage(struct page *page) So at least two was explicit marked inline. Now if that made I change i dunno. Sam - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [IrDA] Fix IrDA build failure
On Mon, Jul 16, 2007 at 03:56:26PM +0400, Evgeniy Polyakov wrote: > Hi Samuel. > > On Mon, Jul 16, 2007 at 02:17:15PM +0300, Samuel Ortiz ([EMAIL PROTECTED]) > wrote: > > This is due to the irda_init fix recently added, where we call __exit > > routines from an __init one. It is a build failure that I didn't catch > > because it doesn't show up when building IrDA as a module. My apologies > > for that. > > What about having all of them __devinit/__devexit? __devinit is used to say that this funtion is used for hotplug so place this in section .init.text only if !HOTPLUG. So with __devinit/__devexit we would see exactly the same in the !HOTPLUG case but only make it harder to trigger since the normal build has HOTPLUG enabled. New version of modpost will btw. catch these kind of errors so also in the x86 case we will be notified - but then only as a warning not as an error. Sam - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] net: fix typo in drivers/net/usb/Kconfig
Replace invisible character with a space. The diff looks like this on my terminal: -Choose this option if you're using a host-to-host cable -with one of these chips. + Choose this option if you're using a host-to-host cable + with one of these chips. Reported by: Massimo Maiurana <[EMAIL PROTECTED]> Signed-off-by: Sam Ravnborg <[EMAIL PROTECTED]> Cc: Massimo Maiurana <[EMAIL PROTECTED]> --- diff --git a/drivers/net/usb/Kconfig b/drivers/net/usb/Kconfig index 3de564b..8dc09a3 100644 --- a/drivers/net/usb/Kconfig +++ b/drivers/net/usb/Kconfig @@ -313,8 +313,8 @@ config USB_KC2190 boolean "KT Technology KC2190 based cables (InstaNet)" depends on USB_NET_CDC_SUBSET && EXPERIMENTAL help - Choose this option if you're using a host-to-host cable - with one of these chips. + Choose this option if you're using a host-to-host cable + with one of these chips. config USB_NET_ZAURUS tristate "Sharp Zaurus (stock ROMs) and compatible" - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] net/hp100: fix section mismatch warning
Fix following section mismatch warning in hp100:
WARNING: drivers/net/hp100.o(.init.text+0x26a): Section mismatch: reference to
.exit.text: (after 'init_module')
The warning says that we use a function marked __exit from a
function marked __init.
This is not good on architectures where we discard __exit section
for drivers that are built-in.
Signed-off-by: Sam Ravnborg <[EMAIL PROTECTED]>
---
Comple tested only.
Note: This warning does not appear with modpost in upstream kernel
but I would like to submit patches for the obvious bugs ASAP
Sam
diff --git a/drivers/net/hp100.c b/drivers/net/hp100.c
index 8118a67..8caa591 100644
--- a/drivers/net/hp100.c
+++ b/drivers/net/hp100.c
@@ -3005,7 +3005,7 @@ static int __init hp100_isa_init(void)
return cards > 0 ? 0 : -ENODEV;
}
-static void __exit hp100_isa_cleanup(void)
+static void hp100_isa_cleanup(void)
{
int i;
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Routing 600+ vlan's via linux problems (looks like arp problems)
On Thu, May 03, 2007 at 10:25:48PM +0200, Øyvind Vågen Jægtnes wrote: > Hi, Hi Øyvind. Forwarding your mail to netdev where the networking people are hanging out. Maybe they can help you. Sam > > We have a one gigabit internet connection that is normally > routed by a hardware juniper router. The drive in this is down > and we need to use a linux machine (Pentium D 3 ghz) as a > temporary router. > Now setting up all the 600 vlans and assigning ip addresses > is no problem. We have testet all by using a laptop, setting up > 600 vlan interfaces on this and running dhcpclient on all. > This worked just fine, all the interfaces got address. > > Now for the real setup. > We closed the mac of the juniper to the network card that > would be connected to the internal LAN, set up the interfaces, > and swapped cables. This worked fine for approximately 100 > of the computers that are connected, but the rest would not > get IP. The connected 100 computers were routed just fine. > > What we think the problem is, is that the arp cache on the > linux router seems strange. It can resolve the MAC for the > 100 clients that actually got through. > For the rest all we see in the arp cache is (incomplete) > > Here is some of the listing for arp -n: > 193.239.155.118 ether 00:0A:E4:59:75:66 C > eth1.1087 > 193.239.154.74 (incomplete) > eth1.1016 > 193.239.155.7ether 00:11:95:D2:3F:FD C > eth1.2002 > 83.143.114.222 (incomplete) > eth1.1305 > 83.143.113.246 ether 00:0B:5D:4B:B8:77 C > eth1.1247 > 83.143.116.126 (incomplete) > eth1.1409 > 83.143.118.114 (incomplete) > eth1.1534 > 193.239.154.210 ether 00:03:0D:2F:1B:7F C > eth1.1050 > 169.254.69.247 ether 00:15:C5:C2:31:6C C > eth1.1262 > 83.143.112.38(incomplete) > eth1.1131 > 83.143.118.18(incomplete) > eth1.1510 > 83.143.112.118 ether 00:11:95:CE:BF:72 C > eth1.1151 > 192.168.1.2 ether 00:0D:88:78:C0:00 C > eth1.2050 > 83.143.117.138 (incomplete) > eth1.1476 > 83.143.116.18(incomplete) > eth1.1382 > 83.143.118.26(incomplete) > eth1.1512 > 83.143.112.6 (incomplete) > eth1.1123 > 193.239.155.62 (incomplete) > eth1.1073 > > `arp -n|wc -l` returns around 350, which is the number of active ports on > the > edge switches... > this number is confirmed by snmp > > I have looked through the source for arp.c but i can't see any immediate > problems. There is no messages in dmesg, kern.log og messages (except for > eth1.vlanid up * 600). > > If anyone know what the problem can be, if this is a bug, or if PSBKC i > would > much appreciate it. > > regards > Øyvind Vågen Jægtnes > +47 96 22 03 08 > [EMAIL PROTECTED] > - > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to [EMAIL PROTECTED] > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Makefile for linux modules
Hi Robert. >I have a makefielt to make several driver modules: > obj-$(CONFIG_FUSION_SPI) += mptbase.o mptscsih.o > mptspi.o > obj-$(CONFIG_FUSION_FC) += mptbase.o mptscsih.o > mptfc.o > obj-m += mptbase.o mptscsih.o mptsas.o > obj-$(CONFIG_FUSION_LAN) += mptlan.o > obj-m += mptctl.o > obj-m += mptcfg.o > obj-m +=mptstm.o The above kbuild file snippet tells us that you are creating a number of modules: mptbase.ko mptscsih.ko mptsas.ko mptlan.ko mptctl.ko mtpcfg.ko and mptstm.ko They are each build from a single .c file. > mptbase-objs := comfunc.o Now you try to include confunc.o in every module. To do so you need to tell kbuild that you are dealing with a module based on composite .o files. That would look like: obj-$(CONFIG_FUSION_PCI) += mptbase-foo.o mtpbase-foo-y := comfunc.o mptbase.o This will result in a module named mtpbase-foo.ko which is hardly what you try to achive. Likewise you will have duplicate symbols in the modules due to comfunc.o being included more than once. The only sane approce here is to compile comfunc.o as an independent module and let the modutils pull in the comfunc (deservers a more specific name) module as needed. So what you need to do is simply: obj-m += comfunc.o And accept this is a module so all symbols that you needs must be properly exported using EXPORT_SYMBOL* Sam - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/4]: powerpc/cell spidernet ethernet driver fixes
On Fri, Aug 11, 2006 at 12:03:37PM -0500, Linas Vepstas wrote: > > The following series of patches implement some fixes and performance > improvements for the Spedernet ethernet device driver. The high point > of the patch series is some code to implement a low-watermark interrupt > on the transmit queue. The bundle of patches raises transmit performance > from some embarassingly low value to a reasonable 730 Megabits per > second for 1500 byte packets. > > Please note that the spider is an ethernet controller that is > integrated into the south bridge, and is thus available only on > Cell-based platforms. > > These have been well-tested over the last few weeks. Please apply. Hi Linas. Just noticed a nit-pick detail. The general rule is to add your Signed-off-by: at the bottom of the patch, so the top-most Signed-of-by: is also the original author whereas the last Signed-of-by: is the one that added this patch to the kernel. Likewise you add Cc: before your Signed-off-by: line. See patches from for example Andrew Morton for examples. Sam - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/6] ehea: makefile
On Wed, Aug 09, 2006 at 10:40:20AM +0200, Jan-Bernd Themann wrote: > Signed-off-by: Jan-Bernd Themann <[EMAIL PROTECTED]> > > > drivers/net/ehea/Makefile |7 +++ > 1 file changed, 7 insertions(+) > > > > --- linux-2.6.18-rc4-orig/drivers/net/ehea/Makefile 1969-12-31 > 16:00:00.0 -0800 > +++ kernel/drivers/net/ehea/Makefile 2006-08-08 23:59:38.083467216 -0700 > @@ -0,0 +1,7 @@ > +# > +# Makefile for the eHEA ethernet device driver for IBM eServer System p > +# > + > +ehea_mod-objs = ehea_main.o ehea_phyp.o ehea_qmr.o ehea_ethtool.o > ehea_phyp.o > +obj-$(CONFIG_EHEA) += ehea_mod.o > + Using -objs is deprecated, please use ehea_mod-y. This needs to be documented and later warned upon which I will do soon. Sam - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Subsystem/prefix in patch submission format
On Sun, Jul 30, 2006 at 03:59:56PM -0700, David Miller wrote: > From: Sam Ravnborg <[EMAIL PROTECTED]> > Date: Mon, 31 Jul 2006 00:45:43 +0200 > > > >From a pure eye-candy perspective it would be nice to use same format > > all over. > > >From Documentation/SubmittingPatches: > > -- > > 12) The canonical patch format > > > > The canonical patch subject line is: > > > > Subject: [PATCH 001/123] subsystem: summary phrase > > -- > > The patch format specifies things that make no sense > when the changes gets into the tree. Especially the > sequence numbers, which are implied by the changes > placement in the relative changeset history. "git am" removes the patch format specifiers when the change are committed. git applymbox replace the patch format specifier with [PATCH]. So: git am=> subsystem: summary phrase git applymbox => [PATCH] subsystem: summary phrase Both variants exists because Linus uses the latter to distingush between what is applied via git tress and what is applied via mail patches. The Signed-off-by: carries same info btw. Sam - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Subsystem/prefix in patch submission format
On Sun, Jul 30, 2006 at 03:28:46PM -0700, David Miller wrote: > From: "Ilpo J?rvinen" <[EMAIL PROTECTED]> > Date: Sun, 30 Jul 2006 14:01:09 +0300 (EEST) > > >[TCP] FRTO: summary here > > This looks perfectly fine. Looking 100 commits back or so it is obvious we have two distinct notations: subsystem: and [subsystem] net related stuff counts for most of the latter but it is used in several other places. >From a pure eye-candy perspective it would be nice to use same format all over. >From Documentation/SubmittingPatches: -- 12) The canonical patch format The canonical patch subject line is: Subject: [PATCH 001/123] subsystem: summary phrase -- Sam - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: tiacx - don't use UTS_RELEASE
On Tue, Jul 04, 2006 at 11:27:27AM +0200, Arjan van de Ven wrote: > On Tue, 2006-07-04 at 02:25 -0700, Andrew Morton wrote: > > On Tue, 04 Jul 2006 11:07:59 +0200 > > Arjan van de Ven <[EMAIL PROTECTED]> wrote: > > > > > patch below removes the use of UTS_RELEASE from the tiacx driver; there > > > is absolutely no reason for a driver to print the kernel version or use > > > the UTS_RELEASE field; in addition this field changes all the time so > > > this causes spurious rebuilds.. > > > > http://www.kernel.org/pub/linux/kernel/people/gregkh/gregkh-2.6/gregkh-04-usb/usb-storage-uname-in-pr-sc-unneeded-message.patch > > did it too. > > > > UTS_RELEASE doesn't change much. It's "2.6.17". > > no but the header that it's in changes all the time iirc, at least it > used to (one of those kbuild regenerated files) Yesterday I pushed a change that splitted include/linux/version.h in two parts. Now include/linux/version.h only contains: #define LINUX_VERSION_CODE 132625 #define KERNEL_VERSION(a,b,c) (((a) << 16) + ((b) << 8) + (c)) And the file wil only be regenerated when the file-content actually changes. And UTS_RELEASE has moved to include/linux/utsrelease.h which contains: #define UTS_RELEASE "2.6.17-g05668381-dirty" This is the file that will change often - at least for git users. But with the patch only users of UTS_RELEASE will be rebuild which is far less than users of version.h. Sam - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [2.6.17-rc6] Section mismatch in drivers/net/ne.o during modpost
> Probably over enthusiastic gcc inlining. gcc 4 will inline functions > that are not declared as inline. That is not so bad, except that some > versions of gcc will ignore a mismatch in function attributes and > inline a __init function into normal text, generating additional > section mismatches. When using -ffunction-sections (or similar) then we ask gcc to put each function in separate sections. In this case it is OK to mix sections. But I agree, gcc should not mix user specified sections. > For a specific example, see > > http://marc.theaimsgroup.com/?l=linux-kernel&m=113824309203482&w=2 That's a good example - thanks! Sam - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [2.6.17-rc6] Section mismatch in drivers/net/ne.o during modpost
On Sun, Jun 11, 2006 at 04:37:31PM +0200, Mikael Pettersson wrote:
> On Sat, 10 Jun 2006 22:38:00 +0200, Sam Ravnborg wrote:
> >> --- linux-2617-rc6.orig/drivers/net/ne.c
> >> +++ linux-2617-rc6/drivers/net/ne.c
> >> @@ -829,7 +829,7 @@ that the ne2k probe is the last 8390 bas
> >> is at boot) and so the probe will get confused by any other 8390 cards.
> >> ISA device autoprobes on a running machine are not recommended anyway. */
> >>
> >> -int init_module(void)
> >> +int __init init_module(void)
> >> {
> >>int this_dev, found = 0;
> >
> >When you anyway touches the driver I suggest to name the function
> >_init, _cleanup and use module_init(), module_cleanup().
>
> Maybe not: in the ne.c driver init_module() is inside #ifdef MODULE,
> so conversion to ne_init() + module_init(ne_init) would be a no-op
> except for making the code larger. In the non-MODULE case Space.c
> calls ne_probe() directly.
The whole purpose of marking a function __init is to place in in a
section that can be discarded after init. This has the added advantage
that it kills off some ugly #ifdef MODULE / #endif as is the case for
ne.c
Even if not discarded then the code cleaniness is preferable to #ifdef /
#endif if purpose is only to save a few bytes.
Shifting to module_init(), module_cleanup() is the only right thing to
do - and the old behaviour is not even documented in LDD3 anymore.
[At least I did not find it last time I searched].
Sam
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [2.6.17-rc6] Section mismatch in drivers/net/ne.o during modpost
> >
> > Not sure how serious this is; the driver seems to work fine later on.
>
> Doesn't look serious. init_module() is not __init, but it calls
> some __init functions and touches some __initdata.
This is the typical case with inconsistent tagging.
> BTW, I would be happy to see some consistent results from modpost
> section checking. I don't see all of these warnings (I see only 1)
> when using gcc 3.3.6. What gcc version are you using?
> Does that matter? (not directed at anyone in particular)
I did not see anyone of them - strange.
I did not dig into it, but objdump -rR ne.o should tell the number of
mismatches with soem carefull checking.
> --- linux-2617-rc6.orig/drivers/net/ne.c
> +++ linux-2617-rc6/drivers/net/ne.c
> @@ -829,7 +829,7 @@ that the ne2k probe is the last 8390 bas
> is at boot) and so the probe will get confused by any other 8390 cards.
> ISA device autoprobes on a running machine are not recommended anyway. */
>
> -int init_module(void)
> +int __init init_module(void)
> {
> int this_dev, found = 0;
When you anyway touches the driver I suggest to name the function
_init, _cleanup and use module_init(), module_cleanup().
Sam
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] vlan: add sysfs support
On Mon, May 01, 2006 at 02:08:34PM -0700, Stephen Hemminger wrote: > Add basic sysfs support for vlan device. It creates an entry in the > vlan pseudo-device to display tag. > /sys/class/net/eth0.5/vlan_id > > It would be nice at some future to have something like: > /sys/class/net/eth0/tags/5 -> ../../eth0.5 > as well. > > There is a race with udev though. The sysfs entry can't be created > until after the the class_device is registered but the registration > doesn't happen until rtnl_unlock after register_netdevice. The class_device > registration causes the hotplug event, so the hotplug user program > might get there before vlan_id is created. > > > --- vlan.orig/net/8021q/Makefile 2006-05-01 12:50:58.0 -0700 > +++ vlan/net/8021q/Makefile 2006-05-01 14:04:56.0 -0700 > @@ -6,7 +6,5 @@ > > 8021q-objs := vlan.o vlan_dev.o Please make this > 8021q-y := vlan.o vlan_dev.o To avoid confusion. > > -ifeq ($(CONFIG_PROC_FS),y) > -8021q-objs += vlanproc.o > -endif > - > +8021q-$(CONFIG_PROC_FS) += vlanproc.o > +8021q-$(CONFIG_SYSFS) += vlan_sysfs.o Sam - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] net drivers: fix section attributes for gcc
Is this pending in your tree Jeff?
It fixes compile regressions with HOTPLUG disabled.
Sam
On Mon, Apr 03, 2006 at 04:28:46PM -0700, Randy.Dunlap wrote:
> From: Randy Dunlap <[EMAIL PROTECTED]>
>
> If CONFIG_HOTPLUG=n, gcc doesn't like some __initdata to be
> const (rodata) and other __initdata not const, so make the
> non-const __initdata const.
>
> gcc errors:
> drivers/net/bnx2.c:66: error: version causes a section type conflict
> drivers/net/starfire.c:338: error: version causes a section type conflict
> drivers/net/typhoon.c:137: error: version causes a section type conflict
> drivers/net/natsemi.c:241: error: version causes a section type conflict
>
> Signed-off-by: Randy Dunlap <[EMAIL PROTECTED]>
Acked-by: Sam Ravnborg <[EMAIL PROTECTED]>
> ---
> drivers/net/bnx2.c |2 +-
> drivers/net/natsemi.c |2 +-
> drivers/net/starfire.c |2 +-
> drivers/net/typhoon.c |2 +-
> 4 files changed, 4 insertions(+), 4 deletions(-)
>
> --- linux-2617-rc1.orig/drivers/net/bnx2.c
> +++ linux-2617-rc1/drivers/net/bnx2.c
> @@ -63,7 +63,7 @@
> /* Time in jiffies before concluding the transmitter is hung. */
> #define TX_TIMEOUT (5*HZ)
>
> -static char version[] __devinitdata =
> +static const char version[] __devinitdata =
> "Broadcom NetXtreme II Gigabit Ethernet Driver " DRV_MODULE_NAME " v"
> DRV_MODULE_VERSION " (" DRV_MODULE_RELDATE ")\n";
>
> MODULE_AUTHOR("Michael Chan <[EMAIL PROTECTED]>");
> --- linux-2617-rc1.orig/drivers/net/natsemi.c
> +++ linux-2617-rc1/drivers/net/natsemi.c
> @@ -238,7 +238,7 @@ static int full_duplex[MAX_UNITS];
> #define NATSEMI_RX_LIMIT 2046/* maximum supported by hardware */
>
> /* These identify the driver base version and may not be removed. */
> -static char version[] __devinitdata =
> +static const char version[] __devinitdata =
>KERN_INFO DRV_NAME " dp8381x driver, version "
>DRV_VERSION ", " DRV_RELDATE "\n"
>KERN_INFO " originally by Donald Becker <[EMAIL PROTECTED]>\n"
> --- linux-2617-rc1.orig/drivers/net/starfire.c
> +++ linux-2617-rc1/drivers/net/starfire.c
> @@ -335,7 +335,7 @@ do { \
>
>
> /* These identify the driver base version and may not be removed. */
> -static char version[] __devinitdata =
> +static const char version[] __devinitdata =
> KERN_INFO "starfire.c:v1.03 7/26/2000 Written by Donald Becker <[EMAIL
> PROTECTED]>\n"
> KERN_INFO " (unofficial 2.2/2.4 kernel port, version " DRV_VERSION ", "
> DRV_RELDATE ")\n";
>
> --- linux-2617-rc1.orig/drivers/net/typhoon.c
> +++ linux-2617-rc1/drivers/net/typhoon.c
> @@ -134,7 +134,7 @@ static const int multicast_filter_limit
> #include "typhoon.h"
> #include "typhoon-firmware.h"
>
> -static char version[] __devinitdata =
> +static const char version[] __devinitdata =
> "typhoon.c: version " DRV_MODULE_VERSION " (" DRV_MODULE_RELDATE ")\n";
>
> MODULE_AUTHOR("David Dillow <[EMAIL PROTECTED]>");
>
>
> ---
> -
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to [EMAIL PROTECTED]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] ipv4: initialize arp_tbl rw lock
On Sat, Apr 08, 2006 at 03:14:04AM -0700, David S. Miller wrote: > Perhaps fs_initcall() would work better. Or if that causes > problems we could create a net_initcall() that sits between > fs_initcall() and device_initcall(). fs_initcall() seems to be used mainly for "init after subsystem" stuff. Within fs/ only pipe.c uses fs_initcall(). If we are going to overload the usage of fs_initcall() even more then we should maybe try to rename it? Sam - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] acenic: fix section mismatches
On Mon, Apr 03, 2006 at 10:27:32AM -0700, Randy.Dunlap wrote:
> On Sun, 2 Apr 2006 23:12:03 +0200 Sam Ravnborg wrote:
>
> > On Mon, Mar 27, 2006 at 02:32:14PM -0800, Randy.Dunlap wrote:
> > > >
> > > > Sam, I am now seeing this error (not a WARNING like the previous ones
> > > > were):
> > > >
> > > > drivers/net/typhoon.c:137: error: version causes a section type conflict
> > > >
> > > > but I don't see a real problem in the driver source file.
> > > > Ideas, anyone?
> > >
> > > This happens for typhoon (above) as well as:
> > >
> > > drivers/net/natsemi.c:241: error: version causes a section type conflict
> >
> > I have taken a look at natsemi.c.
> > Following patch fixes the error:
> >
> > diff --git a/drivers/net/natsemi.c b/drivers/net/natsemi.c
> > index 7826afb..a36a15e 100644
> > --- a/drivers/net/natsemi.c
> > +++ b/drivers/net/natsemi.c
> > @@ -372,7 +372,7 @@ enum pcistuff {
> > static const struct {
> > const char *name;
> > unsigned long flags;
> > -} natsemi_pci_info[] __devinitdata = {
> > +} natsemi_pci_info[] /*__devinitdata*/ = {
> > { "NatSemi DP8381[56]", PCI_IOTYPE },
> > };
> >
> > The reason why gcc barfs over it seems to be conflicting sections
> > between the function natsemi_probe1() where natsemi_pci_info[] is used
> > and the section of natsemi_pci_info.
> > But putting them in same section did not help.
> >
> > I'm a bit puzzeled what is going on. Randy - any insight in this?
>
> How weird. I can't reproduce this at all now. Tried several times.
You need to build with CONFIG_HOTPLUG undefined to see it..
Sam
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] acenic: fix section mismatches
On Mon, Mar 27, 2006 at 02:32:14PM -0800, Randy.Dunlap wrote:
> >
> > Sam, I am now seeing this error (not a WARNING like the previous ones were):
> >
> > drivers/net/typhoon.c:137: error: version causes a section type conflict
> >
> > but I don't see a real problem in the driver source file.
> > Ideas, anyone?
>
> This happens for typhoon (above) as well as:
>
> drivers/net/natsemi.c:241: error: version causes a section type conflict
I have taken a look at natsemi.c.
Following patch fixes the error:
diff --git a/drivers/net/natsemi.c b/drivers/net/natsemi.c
index 7826afb..a36a15e 100644
--- a/drivers/net/natsemi.c
+++ b/drivers/net/natsemi.c
@@ -372,7 +372,7 @@ enum pcistuff {
static const struct {
const char *name;
unsigned long flags;
-} natsemi_pci_info[] __devinitdata = {
+} natsemi_pci_info[] /*__devinitdata*/ = {
{ "NatSemi DP8381[56]", PCI_IOTYPE },
};
The reason why gcc barfs over it seems to be conflicting sections
between the function natsemi_probe1() where natsemi_pci_info[] is used
and the section of natsemi_pci_info.
But putting them in same section did not help.
I'm a bit puzzeled what is going on. Randy - any insight in this?
Sam
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] acenic: fix section mismatches
On Mon, Mar 27, 2006 at 02:32:14PM -0800, Randy.Dunlap wrote: > > > > > > Nope, only with HOTPLUG enabled. I can try without also (but > > > disabling it is a pain :). > > > > ugh, FW_LOADER is the beast (not CONFIG_HOTPLUG itself). > > > > Sam, I am now seeing this error (not a WARNING like the previous ones were): Can you send me your .config. Tried but did not manage to get rid of FW_LOADER. OK - did not try hard though. Sam - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] acenic: fix section mismatches
On Mon, Mar 27, 2006 at 12:26:12PM -0800, Randy.Dunlap wrote: > From: Randy Dunlap <[EMAIL PROTECTED]> > > Fix section mismatches in acenic driver: > WARNING: drivers/net/acenic.o - Section mismatch: reference to > .init.data:tigon2FwText from .text between 'acenic_probe_one' (at offset > 0x2409) and 'ace_interrupt' > WARNING: drivers/net/acenic.o - Section mismatch: reference to > .init.data:tigon2FwRodata from .text between 'acenic_probe_one' (at offset > 0x2422) and 'ace_interrupt' > > Signed-off-by: Randy Dunlap <[EMAIL PROTECTED]> I'm glad to see these fixes appear. Did you manage to compile with and without HOTPLUG enabled? This is important since HOTPLUG redefined __dev* to nothing => no section mismatchs. Sam - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Discourage duplicate symbols in the kernel? [Was: Intel I/O Acc...]
On Sun, Mar 05, 2006 at 12:09:33AM -0800, Andrew Morton wrote:
> > +
> > +static inline u8 read_reg8(struct cb_device *device, unsigned int offset)
> > +{
> > + return readb(device->reg_base + offset);
> > +}
>
> These are fairly generic-sounding names. In fact the as-yet-unmerged tiacx
> wireless driver is already using these, privately to
> drivers/net/wireless/tiacx/pci.c.
Do we in general discourage duplicate symbols even if they are static?
[ppc64, allmodconfig]
$> nm vmlinux | fgrep ' t ' | awk '{print $3}' | sort | uniq -dc
2 .add_bridge
2 .base_probe
2 .c_next
2 .c_start
2 .c_stop
3 .cpu_callback
2 .default_open
2 .default_read_file
2 .default_write_file
2 .dev_ifsioc
2 .do_open
4 .dst_output
2 .dump_seek
2 .dump_write
2 .elf_core_dump
2 .elf_map
2 .exact_lock
2 .exact_match
2 .exit_elf_binfmt
2 .fill_note
2 .fill_prstatus
2 .fillonedir
2 .fini
2 .fixup_one_level_bus_range
5 .init
8 .init_once
3 .iommu_bus_setup_null
3 .iommu_dev_setup_null
2 .klist_devices_get
2 .klist_devices_put
2 .load_elf_binary
2 .load_elf_interp
2 .load_elf_library
3 .m_next
3 .m_start
3 .m_stop
2 .maydump
3 .modalias_show
2 .next_device
3 .notesize
2 .padzero
2 .raw_ioctl
2 .s_next
2 .s_show
2 .s_start
2 .s_stop
2 .seq_next
2 .seq_show
2 .seq_start
2 .seq_stop
2 .set_brk
2 .setkey
2 .state_show
2 .state_store
2 .store_uevent
2 .u3_ht_cfg_access
2 .u3_ht_read_config
2 .u3_ht_write_config
2 .writenote
3 __initcall_init
2 __setup_netdev_boot_setup
2 __setup_str_netdev_boot_setup
If I did a make allyesconfig the result looks much more scary.
Sam
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
de620: fix section mismatch warning
In latest -mm de620 gave following warning:
WARNING: drivers/net/de620.o - Section mismatch: reference to \
.init.text:de620_probe from .text between 'init_module' (at offset \
0x1682) and 'cleanup_module'
init_module() call de620_probe() which is declared __init.
Fix is to declare init_module() __init too.
Signed-off-by: Sam Ravnborg <[EMAIL PROTECTED]>
---
Compile tested with allmodconfig on amd64.
diff --git a/drivers/net/de620.c b/drivers/net/de620.c
index 0069f5f..22fc5b8 100644
--- a/drivers/net/de620.c
+++ b/drivers/net/de620.c
@@ -1012,7 +1012,7 @@ static int __init read_eeprom(struct net
#ifdef MODULE
static struct net_device *de620_dev;
-int init_module(void)
+int __init init_module(void)
{
de620_dev = de620_probe(-1);
if (IS_ERR(de620_dev))
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
wan: fix section mismatch warning in sbni
In latest -mm sbni gives following warning:
WARNING: drivers/net/wan/sbni.o - Section mismatch: reference to\
.init.data: from .text between 'init_module' (at offset 0x14ef) and \
'cleanup_module'
The warning is caused by init_module() calling a function declared
__init.
Declare init_module() __init too to fix warning.
Signed-off-by: Sam Ravnborg <[EMAIL PROTECTED]>
---
Compile tested with allmodconfig on amd64.
diff --git a/drivers/net/wan/sbni.c b/drivers/net/wan/sbni.c
index db2c798..175ba13 100644
--- a/drivers/net/wan/sbni.c
+++ b/drivers/net/wan/sbni.c
@@ -1495,8 +1495,7 @@ module_param(skip_pci_probe, bool, 0);
MODULE_LICENSE("GPL");
-int
-init_module( void )
+int __init init_module( void )
{
struct net_device *dev;
int err;
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] net: do not export inet_bind_bucket_create twice
inet_bind_bucket_create was exported twice. Keep the export in the file where inet_bind_bucket_create is defined. Signed-off-by: Sam Ravnborg <[EMAIL PROTECTED]> --- I've noticed double export while playing with modpost. I did not see any ill-effect of it - but modpost may soon complain. EXPORT_SYMBOL(sysctl_local_port_range) in same file is also misplaced, but the fix is less obvious. Sam diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c index 6ea3539..7e7c40e 100644 --- a/net/ipv4/tcp_ipv4.c +++ b/net/ipv4/tcp_ipv4.c @@ -1845,7 +1845,6 @@ void __init tcp_v4_init(struct net_proto } EXPORT_SYMBOL(ipv4_specific); -EXPORT_SYMBOL(inet_bind_bucket_create); EXPORT_SYMBOL(tcp_hashinfo); EXPORT_SYMBOL(tcp_prot); EXPORT_SYMBOL(tcp_unhash); - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
